Thanks Barry for the detailed comments. It looks like some of your comments overlap two other comment sets in the works, for which I've responded to the list with proposed changes, but are awaiting responses.
Should I try to fold all 3 sets of changes into a new 13 draft? or would it be easier for you if I made 2 separate passes? -vince On Mon, Jul 14, 2014 at 12:31 PM, Barry Leiba <[email protected]> wrote: > Hey, PAWS folk. > As Pete has put the subject document on the 7 August telechat agenda, > though he still has it in "Waiting for AD Go Ahead" state, I decided > to review it now, rather than wait. I've found quite a list of > issues, from large to small, which I've sorted below into three > categories: > > "DISCUSS" are those that I would put in a DISCUSS ballot during IESG > Evaluation. I think they're errors in the document that need to be > corrected, or else things that need to be discussed and resolved. > > The most important in this category, and the only one I think will be > difficult to resolve, is how to specify the JSON payload formally. > > "COMMENT -- substantive" are those that I would put in the > non-blocking comments, but that I think are important to deal with, > and that I'd hope we'd have a discussion about if the authors think a > discussion is necessary. > > "COMMENT -- minor" are minor editorial issues that I think would be > better corrected, but that I'm not going to grump about further. > Discussion is always welcome, but should be pretty much unnecessary > for these. > > I hope it's useful to get these comments earlier, rather than later. > And, so, here they are, below. I've offered suggested text where I > could, and I hope that helps as well. > > Barry > > ======================= DISCUSS ======================= > > -- Section 4 -- > > o Device Registration (Section 4.3) MAY be used by the Master Device > and MAY be implemented by the Database, either as a separate > component or as part of the Available Spectrum Query (Section 4.4) > component. > > I don't think the first MAY is correct. If the database requires > registration, it is *not* optional for the Master Device to use it. I > think this needs some rework. The same is true with "Device > Validation". > > This list also mixes "used", "supported", and "implemented", without > making all the issues clear. If something MAY be used, does that mean > it's optional to implement? If it MUST be supported, what does that > say about its use? And what's the difference between "supported" and > "implemented"? > > I strongly suggest that, if you're going to say that, you make it all > clear, as to both implementation/support and use. The note after the > list that says that some things are obvious... nah, one of it is > obvious. > > -- Section 4.1 -- > > A Database MAY indicate that its URI will be changing by including > the URI of one or more alternate databases (See DbUpdateSpec > (Section 5.7)) in its responses to a Device. Before a Database > ceases operation, for example, it MUST include DbUpdateSpec in its > responses to notify Devices. > > I don't understand how the MAY and MUST are consistent with each > other. Can you explain or fix it? > > -- Section 4.1.1 -- > > When a Listing Server is used, the > Device can save the database list and SHOULD contact the Database > Listing Server periodically to update its list. The time between > such updates MUST be no longer than one week > > Here's another 2119 conflict. It doesn't make sense to have a MUST > for the time period if doing it at all is a SHOULD. Please sort this > one out as well. > > -- Section 4.2 > > A Master Device SHOULD use the initialization procedure to exchange > capability information with the Database whenever the Master Device > powers up or initiates communication with the Database. > > But in Section 4 you said "Initialization (Section 4.2) MAY be used by > the Master Device," and now you're saying "SHOULD"? You see why I'm > on about the 2119 inconsistencies? > > When a Master Device is > configured manually with these parameterized-rule values, it does not > need to use the initialization procedure. > > Dos that mean that when it's not configured with them, it *does* need > to? In other words, is this really a "MUST use initialization unless > it's pre-configured with the values" thing? > > -- Section 4.2.1 -- > > deviceDesc: The DeviceDescriptor (Section 5.2) for the Device is > REQUIRED. If the Database does not support the device or any of > the rulesets specified in the device descriptor, it MUST return an > error with the UNSUPPORTED (Table 1) code in the error response. > If the device descriptor does not contain any ruleset IDs, the > Database SHOULD return a list of RulesetInfo (Section 5.6) > parameters for each ruleset it supports at the specified location. > > What this says to me is that the Master Device uses an empty list of > rulesets to query the list of supported rulesets. So: how does this > work if the Database does not return its list of rulesets? > Presumably, the Master Device did this because it needs the list, and > then it didn't get it. What does it do now? > > Also, this is worded a bit confusingly. The list is REQUIRED, so it's > not true that the Database SHOULD return a list. What you mean to > tell us is what the Database SHOULD include in the list. I think it's > really awkward to try to say that here in the query, rather than in > the next section in the response. > > Try this: > > OLD (Section 4.2.1) > deviceDesc: The DeviceDescriptor (Section 5.2) for the Device is > REQUIRED. If the Database does not support the device or any of > the rulesets specified in the device descriptor, it MUST return an > error with the UNSUPPORTED (Table 1) code in the error response. > If the device descriptor does not contain any ruleset IDs, the > Database SHOULD return a list of RulesetInfo (Section 5.6) > parameters for each ruleset it supports at the specified location. > NEW > deviceDesc: The DeviceDescriptor (Section 5.2) for the Device is > REQUIRED. If the device descriptor does not contain any ruleset > IDs, the Master Device is asking the Database to return a list of > RulesetInfo (Section 5.6) parameters for each ruleset it supports > at the specified location. > END > > OLD (Section 4.2.2) > rulesetInfos: A list of RulesetInfo (Section 5.6) parameters MUST be > included in the response. Each RulesetInfo parameter corresponds > to a ruleset supported by the Database and is applicable to the > location specified in the INIT_REQ (Section 4.2.1) message. If > the Device included a list of ruleset IDs in the DeviceDescriptor > parameter of its INIT_REQ message, each RulesetInfo parameter in > the response MUST match one of the specified ruleset IDs. > NEW > rulesetInfos: A list of RulesetInfo (Section 5.6) parameters MUST be > included in the response. Each RulesetInfo parameter corresponds > to a ruleset supported by the Database and is applicable to the > location specified in the INIT_REQ (Section 4.2.1) message. > > If the Device included a list of ruleset IDs in the DeviceDescriptor > parameter of its INIT_REQ message, each RulesetInfo parameter in > the response MUST match one of the specified ruleset IDs. > > If the DeviceDescriptor did not contain any ruleset IDs, the > Database SHOULD include in the list the RulesetInfo parameters for > each ruleset it supports at the specified location. > > If the Database does not support the device or any of the rulesets > specified in the DeviceDescriptor, it MUST instead return an > error with the UNSUPPORTED (Table 1) code in the error response. > END > > -- Section 4.3.2 -- > > If the Database accepts the > registration for none of the rulesets it supports, the Database MUST > return the NOT_REGISTERED error (See Error Codes (Section 5.17)). > > I can't follow this sentence at all. Do you mean, "If the Database > does not accept the registration for any of the rulesets" ? Or do you > mean something else? Can you re-word this, please? > > -- Section 5.8 -- > > name: The display name for a database. It MAY contain UTF-8. > > What about all the other strings whose content format is unspecified? > Are they allowed to contain UTF-8? If not, what *do* they contain? > > I see that the Gen-ART review mentioned this also, and the response > will be to specify that all strings are encoded in UTF-8. Good. > > -- Section 6 -- > How is the reference to JSON Schema [I-D.zyp-json-schema] not > normative, when the 6.x subsections quite depend upon the > understanding of the schema format that it describes? > > I see that the Gen-ART review mentioned this also, but the response is > quite troubling. No, the mechanism you're using is not sufficiently > self explanatory. I understand it -- I do -- but I had to figure it > out (I decided not to go look at the Zyp draft), and we can't assume > that arbitrary implementors will get it right. We need a well defined > way to explain how JSON objects are constructed. And, unfortunately, > the JSON working group chose *not* to take on a work item for this > (which I was pushing for). > > We have at least three proposals (apart from Zyp, there's Newton > (draft-newton-json-content-rules), and there's the mechanism that I > proposed, which is used in Section 6.2 of RFC 7071), but nothing > that's being taken forward as a standard yet. > > -- Section 6.6.2 -- > The schema is missing the optional databaseChange property. > > -- Section 6.7.1 -- > The schema is missing the optional masterDeviceDesc property. > > -- Section 6.8.11 -- > The schema says that both properties are optional, but Section 5.14 > says that they're both required. > > -- Section 6.8.13 -- > The schema says that rulesetInfo is optional, but Section 5.9 says > that it's required. > > -- Section 7 -- > > The Database MAY redirect a PAWS request by returning a HTTP 3xx > response (as defined by HTTP/1.1 [RFC2616]). > ... > Since the Device may communicate > with a Database (which it authenticated) without user interaction, > when the response code is 301 (Moved Permanently), the Device MAY > redirect without asking a user for confirmation (note that this > represents an exception to the HTTP/1.1 [RFC2616] requirements for > HTTP POST methods). > > The references to RFC2616 need to change to RFC7231, now that 2616 is > obsolete. > I see that the Gen-ART review mentioned this also, and you plan to > change the reference. This should help you get that right: > > The change to 7231 also means that the note about the second citation > can change. 2616 said this about redirecting a POST: > > --- RFC 2616 --- > If the 301 status code is received in response to a request other > than GET or HEAD, the user agent MUST NOT automatically redirect the > request unless it can be confirmed by the user, since this might > change the conditions under which the request was issued. > ---------------- > > That advice has changed in 7231: > > --- RFC 7231 --- > Automatic redirection needs to done with > care for methods not known to be safe, as defined in Section 4.2.1, > since the user might not wish to redirect an unsafe request. > ---------------- > > In my view, that means that the parenthesized note in the quote above > can be removed from this document. I also suggest using a section > number in the citation, as you're citing a big document. So... > > OLD > The Database MAY redirect a PAWS request by returning a HTTP 3xx > response (as defined by HTTP/1.1 [RFC2616]). > NEW > The Database MAY redirect a PAWS request by returning a HTTP 3xx > response (as defined by HTTP/1.1 Semantics and Content [RFC7231], > Section 6.4). > END > > OLD > Since the Device may communicate > with a Database (which it authenticated) without user interaction, > when the response code is 301 (Moved Permanently), the Device MAY > redirect without asking a user for confirmation (note that this > represents an exception to the HTTP/1.1 [RFC2616] requirements for > HTTP POST methods). > NEW > Since the Device may communicate > with a Database (which it authenticated) without user interaction, > when the response code is 301 (Moved Permanently), the Device MAY > redirect without asking a user for confirmation, even though it > is in response to an HTTP POST request. > END > > -- Section 8.2 -- > > The parameter name SHOULD be lowerCamelCase. > > Why "SHOULD"? How does it affect interoperability? Wouldn't > "Parameter names use lowerCamelCase by convention." work fine? > > -- Section 9 -- > First, and most importantly, the IANA review interpreted your text to > say that the registration policies are Expert Review; I interpret your > text to say that they're Specification Required. Yet Vince replied to > the IANA review without correcting that. Which policy do you intend? > If it is Specification Required, you need to clear that up with IANA. > Or just deal with my other comments in this section, and that might > happen anyway. > > The text in this section seems to be commonly copied from somewhere. > Perhaps you could tell me whence you copied it, so we can make an > effort to stop people from doing that. > > The first paragraph is entirely unnecessary: please just remove it. > > The second paragraph and the list is fine, and the third is mostly OK, > except for the URI. You'll need to ask IANA how they want to receive > requests (by email or through a web URI), and specify that in > paragraph 3 instead of what's there now. > > The fourth paragraph really needs to be entirely re-thought: it is not > how IANA does expert review in the first place, and we shouldn't be > telling IANA how to run their process in the second place. > > The fourth paragraph should say that all registries use the > Specification Required policy [RFC5226], with a Designated Expert > appointed by the IESG. It should note that instructions to the DE as > to what she should look for and consider in evaluating a request are > given below in the descriptions of each registry. It should say that > the DE should take advice from the community through the designated > mailing list, and that is why the registrant should post to the > mailing list before formally requesting the registration from IANA. > All the rest of the mechanics are part of IANA's process, and should > not be specified here. > > One question: does it make sense for the review list to be a separate > list, or couldn't the WG mailing list be reused for that purpose? > > > ======================= COMMENT -- substantive ======================= > > General: > You appear to sometimes use "parameter" to refer to a structure (such > as GeoLocation) and also the fields within the parameter (see Section > 5.1, for example). I find this confusing, and suggest that you do not > use the same term for both. > > -- Section 3 -- > In step 4, should this not say a word or two about what message it > responds with? Might it respond, "To be, or not to be; that is the > question?" Or is there an expected sort of response? > > Step 5 is the first mention of registration. A reference to where > that process is explained would be useful here (as this step doesn't > seem to fit into the flow). > > In step 7, this is the first mention of a Slave Device, so referring > to "the Slave Device" doesn't make sense. Maybe you want something > like, "If a Slave Device has made a request to the Master Device, the > Master Device may verify..." ? > > It's not clear how step 7 relates to steps 6 and 8. > > In step 10, might it also send an error, refusal, or whatever? Or are > those all considered acknowledgment messages? > > Hm, it seems that steps 5 and 7 are actually explained in the > paragraph after the numbered steps. So do they really belong in the > numbered steps at all? > > -- Section 5.1 -- > > point: If present, it indicates that the GeoLocation represents a > point. > ... > region: If present, it indicates that the GeoLocation represents a > region. > > These make it sound as though these are boolean. It took me a bit to > realize that the values of each of these is actually a structure. I > think you mean this: > > NEW > point: If present, it specifies the GeoLocation as a point. > ... > region: If present, it specifies the GeoLocation as a region. > END > > -- Section 8.3 -- > Please get rid of the MAY in the first sentence. > > If an > appropriate category does not exist, it can use values in a different > range. > > What is the antecedent for "it"? It looks like it's "an appropriate > category", but that's clearly not right. Please fix. > > > ======================= COMMENT -- minor ======================= > > A small point in the header: > "iconectiv (formerly Telcordia Interconnection Solutions)" > I think it's fine to put that in the "Authors' Addresses" section, but > the header should just say "iconectiv". I realize that takes a little > inconvenient massaging of the XML, and probably isn't worth the > trouble for the I-D. Might just put that in an RFC Editor note.... > > General: > I like that your citations include the document names, as well as the > RFC numbers; thanks! It would be better if you put quotation marks > around the document names, to distinguish them from being part of your > text. > > -- Section 1 -- > > The document describes the use of HTTP/TLS as > transport for the protocol. > > This hits what's rather a peeve of mine: HTTP is not a transport > protocol; the transport protocol you're using is TCP. > > I'd be happier if you said something like this: > > OLD > This specification defines an extensible protocol to obtain available > spectrum from a geospatial database by a device with geo-location > capability. > NEW > This specification defines an extensible protocol, built on top of > HTTP and TLS, to obtain available spectrum from a geospatial database > by a device with geo-location capability. > END > > ...and then eliminate that last sentence. See also comments on > Section 4 and Section 7. > > -- Section 2.2 -- > > I suggest this: > > OLD > EIRP: Effective isotropically radiated power > ETSI: European Telecommunications Standards Institute > FCC: Federal Communications Commission > NEW > ETSI: European Telecommunications Standards Institute <http://etsi.org > > > FCC: The U.S. Federal Communications Commission <http://fcc.gov> > END > > ... and then in Section 9.2.2.7, spell out EIRP, as it's only used in > that one place. > > -- Section 3 -- > Little grammar nit: > 1. The Master Device obtains (statically or dynamically) the URI > for a Database appropriate for its location to send subsequent > PAWS messages. > > Make it "location, to which to send". > > -- Section 3.1 -- > > These > requirements may be complex and involve device behavior that are not > easily parameterized. > > "Device behaviour" is singular, so "that is not", please. > > -- Section 4 -- > > Usage pet peeve: please change "comprise" to "compose", in the one > place it's used. > > HTTPS Binding (Section 7) describes the use of > HTTPS (HTTP Over TLS [RFC2818]) for transporting PAWS messages and > optional device authentication. > > Please use "transferring", instead of "transporting", to avoid calling > HTTP a transport. > > -- Section 4.1.1 -- > > TBD Define message format > > You missed something here. Jus' sayin'. > > -- Section 4.4.1 -- > > Rulesets may mandate that it be the Device's > current location or allow it to be an anticipated location. > > This is probably as good a place as any to ask this: is there any > validation of the geo-location at all? If not, can such a mandate > have any real teeth? What is this *really* saying? > > -- Section 4.4.5 -- > The table says that "location" is required, but the text says it's not > always required. The table should say "see description". > > -- Section 4.5 -- > > Typically, a Slave Device needs a Master Device to ask the Database > on its behalf for available spectrum > > "Typically"? Isn't that the *definition* of a Slave Device? > > -- Section 5.5 -- > > All contact information MUST be expressed using the structure defined > by the vCard Format Specification [RFC6350]. > > I think it's best to include both references here, like this: > > NEW > All contact information MUST be expressed using the structure defined > by the vCard Format Specification [RFC6350], encoded in JSON [RFC 7095]. > END > > -- Section 5.7 -- > The subsections explain what additional "data" field each error might > use, but there are only sections for the ones that use the "data" > field. One has to assume that the absence of a subsection means that > the "data" field is not used for that error. It would be better to > say that in the table. I suggest adding one sentence to each error > description in the table except for -104, -105, and -201, as follows: > "This error does not use any additional data." > > -- Section 5.11 -- > The third table should be labelled "SpectrumProfilePoint". > > -- Section 5.12 -- > The second table should be labelled "SpectrumProfilePoint". > > -- Section 5.17.1 -- > > When the error code is OUTSIDE_COVERAGE, the Database MAY include an > ErrorData element within its as the "data" field > > You're missing the words "Error response" after "within its". > > -- Section 7 -- > > This section describes the use of HTTP over TLS (HTTPS) HTTP Over TLS > [RFC2818] as the transport mechanism for the PAWS protocol. > > As I've noted earlier, please say "as the transfer mechanism". Also, > it's really weird to describe the document in the same words as the > document title. I suggest just deleting the document title in this > case. This also applies to Section 10.1. > > -- Section 9.1.2 -- > The first paragraph was obviously written before the ETSI info was > added. Please adjust it accordingly. (I see that the Gen-ART review > mentioned this too, and it will be fixed.) > > You might add instructions to the DE with respect to handling > prefixes. Suppose, for example, I wanted to register something > starting with "fcc". Or perhaps with the initials of an organization > that hasn't yet registered anything, but reasonably might. > > -- Section 10.1 -- > The doubled "HTTP over TLS" comment applies here. Also, it's not at > all clear, when you say "these checks MAY be omitted," exactly what > checks you're saying may be omitted. > > In particular, the > validation path of the certificate must end in one of the client's > trust anchors, even if that trust anchor is the Database certificate > itself. A Master Device should allow for the fact that a Database > can change its certificate authorities (CAs) over time. > > Just a question here: did the WG consider DANE at all? > > Another question: is it really necessary to cover this stuff here? > Isn't it in 2818? > > -- Section 10.3 -- > > Using HTTP over TLS, messages protected by appropriate cypher suites > are also protected from eavesdropping or otherwise access by > unauthorized parties en route. > > What does "or otherwise access" mean? Apart from the grammar problem: > it would be better to say something substantive here, rather than > something vague. > > -- Section 11 -- > I wouldn't usually comment on a "Contributors" section, but it seems > odd to me to cite abandoned and long-expired drafts as references. It > might be better just to include the draft names, without making them > references. No? You're not actually suggesting that people read > them, just thanking the authors for their contributions. > > ============================================== > > _______________________________________________ > paws mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/paws > -- -vince
_______________________________________________ paws mailing list [email protected] https://www.ietf.org/mailman/listinfo/paws
