-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate data before calling NetService.dictionary #86
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -395,10 +395,7 @@ import Foundation | |
|
||
var txtRecord: [String: String] = [:] | ||
if let txtRecordData = netService.txtRecordData() { | ||
let dict = NetService.dictionary(fromTXTRecord: txtRecordData) | ||
for (key, data) in dict { | ||
txtRecord[key] = String(data: data, encoding:String.Encoding.utf8) | ||
} | ||
txtRecord = dictionary(fromTXTRecord: txtRecordData) | ||
} | ||
|
||
var hostName:String = "" | ||
|
@@ -436,4 +433,36 @@ import Foundation | |
return nil | ||
} | ||
|
||
fileprivate static func dictionary(fromTXTRecord txtData: Data) -> [String: String] { | ||
|
||
var result = [String: String]() | ||
var data = txtData | ||
|
||
while !data.isEmpty { | ||
// The first byte of each record is its length, so prefix that much data | ||
let recordLength = Int(data.removeFirst()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably covered by the spec or data structure, but just in case: it's not possible for the length to be greater than one byte, right? |
||
guard data.count >= recordLength else { return [:] } | ||
let recordData = data[..<(data.startIndex + recordLength)] | ||
data = data.dropFirst(recordLength) | ||
|
||
guard let record = String(bytes: recordData, encoding: .utf8) else { return [:] } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like we now have 3 possible return values from this method:
Previously I think if there were no records at all then the final JSON response looks like These guards might be a good spot to do some debug printing too. |
||
// The format of the entry is "key=value" | ||
// (According to the reference implementation, = is optional if there is no value, | ||
// and any equals signs after the first are part of the value.) | ||
// `ommittingEmptySubsequences` is necessary otherwise an empty string will crash the next line | ||
let keyValue = record.split(separator: "=", maxSplits: 1, omittingEmptySubsequences: false) | ||
let key = String(keyValue[0]) | ||
// If there's no value, make the value the empty string | ||
switch keyValue.count { | ||
case 1: | ||
result[key] = "" | ||
case 2: | ||
result[key] = String(keyValue[1]) | ||
default: | ||
fatalError() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How "fatal" is this - does it stackdump the entire application, or will Cordova pass this back as a failed promise? If it does dump, what do you think about catching and returning to Cordova with a failure message so the developer can handle it? |
||
} | ||
} | ||
|
||
return result | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to get stuck inside this
while
? If there is a timeout, how long does it take? We have to be aware of (and prevent if possible) long-running Cordova requests.