Issue 95310043: Flesh out error responses in RFC
|
Created:
9 years, 11 months ago by Al Cutter Modified:
9 years, 11 months ago CC:
ctlog-opensource-review_google.com Visibility:
Public. |
DescriptionFlesh out error responses in RFC
Patch Set 1 #
Total comments: 13
Patch Set 2 : Fixes #Patch Set 3 : Remove RFC2616 'quote' #MessagesTotal messages: 11
https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml File doc/rfc6962-bis.xml (right): https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode815 doc/rfc6962-bis.xml:815: Upon receipt of a <spanx style="verb">400 Bad Request</spanx> response, the client MUST NOT retry the request without modification. From this sentence it's unclear 400 is the response the server should send to indicate malformed requests. I suggest clarifying this at the beginning of the sentence and removing the hard requirements - rather stating the client should re-phrase the request based on input from the error message. https://codereview.appspot.com/95310043/diff/1/doc/rfc6962.xml File doc/rfc6962.xml (right): https://codereview.appspot.com/95310043/diff/1/doc/rfc6962.xml#newcode665 doc/rfc6962.xml:665: If the log is unable to process a client's request, it MUST return an HTTP response code of 4xx/5xx (see <xref target="RFC2616"/>), and, in place of the responses outlined in the subsections below, the body SHOULD be a JSON structure containing at least the following field: I don't think you're meant to change this file, only the -bis one.
Sign in to reply to this message.
https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml File doc/rfc6962-bis.xml (right): https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode798 doc/rfc6962-bis.xml:798: In the case of a malformed request, the string SHOULD provide sufficient detail for the error to be rectified. If you're intending a paragraph break here, you need to wrap it in <t> or something. https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode809 doc/rfc6962-bis.xml:809: "error_message": "'start' cannot be > 'end'", '>' in english, please. https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode815 doc/rfc6962-bis.xml:815: Upon receipt of a <spanx style="verb">400 Bad Request</spanx> response, the client MUST NOT retry the request without modification. Isn't this just restating RFC 2616: "10.4.1 400 Bad Request The request could not be understood by the server due to malformed syntax. The client SHOULD NOT repeat the request without modifications." https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode821 doc/rfc6962-bis.xml:821: </t> Why is there an empty paragraph here? https://codereview.appspot.com/95310043/diff/1/doc/rfc6962.xml File doc/rfc6962.xml (right): https://codereview.appspot.com/95310043/diff/1/doc/rfc6962.xml#newcode665 doc/rfc6962.xml:665: If the log is unable to process a client's request, it MUST return an HTTP response code of 4xx/5xx (see <xref target="RFC2616"/>), and, in place of the responses outlined in the subsections below, the body SHOULD be a JSON structure containing at least the following field: On 2014/05/12 15:28:39, Eran wrote: > I don't think you're meant to change this file, only the -bis one. Eran is correct.
Sign in to reply to this message.
https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml File doc/rfc6962-bis.xml (right): https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode798 doc/rfc6962-bis.xml:798: In the case of a malformed request, the string SHOULD provide sufficient detail for the error to be rectified. On 2014/05/16 10:15:13, Ben Laurie (Google) wrote: > If you're intending a paragraph break here, you need to wrap it in <t> or > something. <t> can't be a child of <t> apparently, I've wrapped it in a <list> for want of a better solution. https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode809 doc/rfc6962-bis.xml:809: "error_message": "'start' cannot be > 'end'", On 2014/05/16 10:15:13, Ben Laurie (Google) wrote: > '>' in english, please. Done. https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode815 doc/rfc6962-bis.xml:815: Upon receipt of a <spanx style="verb">400 Bad Request</spanx> response, the client MUST NOT retry the request without modification. On 2014/05/16 10:15:13, Ben Laurie (Google) wrote: > Isn't this just restating RFC 2616: > > "10.4.1 400 Bad Request > > The request could not be understood by the server due to malformed > syntax. The client SHOULD NOT repeat the request without > modifications." yes. https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode821 doc/rfc6962-bis.xml:821: </t> On 2014/05/16 10:15:13, Ben Laurie (Google) wrote: > Why is there an empty paragraph here? XML induced blindness. Removed.
Sign in to reply to this message.
https://codereview.appspot.com/95310043/diff/1/doc/rfc6962.xml File doc/rfc6962.xml (right): https://codereview.appspot.com/95310043/diff/1/doc/rfc6962.xml#newcode665 doc/rfc6962.xml:665: If the log is unable to process a client's request, it MUST return an HTTP response code of 4xx/5xx (see <xref target="RFC2616"/>), and, in place of the responses outlined in the subsections below, the body SHOULD be a JSON structure containing at least the following field: On 2014/05/16 10:15:13, Ben Laurie (Google) wrote: > On 2014/05/12 15:28:39, Eran wrote: > > I don't think you're meant to change this file, only the -bis one. > > Eran is correct. Rats, reverted.
Sign in to reply to this message.
https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml File doc/rfc6962-bis.xml (right): https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode815 doc/rfc6962-bis.xml:815: Upon receipt of a <spanx style="verb">400 Bad Request</spanx> response, the client MUST NOT retry the request without modification. On 2014/05/16 10:42:38, Al Cutter wrote: > On 2014/05/16 10:15:13, Ben Laurie (Google) wrote: > > Isn't this just restating RFC 2616: > > > > "10.4.1 400 Bad Request > > > > The request could not be understood by the server due to malformed > > syntax. The client SHOULD NOT repeat the request without > > modifications." > > yes. My point was that if 2616 says it, why are we? And why this particular error code and not others?
Sign in to reply to this message.
On 2014/05/16 10:50:18, Ben Laurie (Google) wrote: > https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml > File doc/rfc6962-bis.xml (right): > > https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode815 > doc/rfc6962-bis.xml:815: Upon receipt of a <spanx style="verb">400 Bad > Request</spanx> response, the client MUST NOT retry the request without > modification. > On 2014/05/16 10:42:38, Al Cutter wrote: > > On 2014/05/16 10:15:13, Ben Laurie (Google) wrote: > > > Isn't this just restating RFC 2616: > > > > > > "10.4.1 400 Bad Request > > > > > > The request could not be understood by the server due to malformed > > > syntax. The client SHOULD NOT repeat the request without > > > modifications." > > > > yes. > > My point was that if 2616 says it, why are we? And why this particular error > code and not others? Well, there's currently something talking to the prod logs which definitely does not conform to the section of 2616 you pointed out so it seemed worthwhile reinforcing it. We've also seen on numerous occasions that the DigitalSignature has caused people confusion, so I have some reason to doubt that people actually look at the referenced RFCs when they're reading this one. If you feel strongly about it I'm happy to remove it though.
Sign in to reply to this message.
On 19 May 2014 10:50, <alcutter@google.com> wrote: > On 2014/05/16 10:50:18, Ben Laurie (Google) wrote: >> >> https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml >> File doc/rfc6962-bis.xml (right): > > > > https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode815 >> >> doc/rfc6962-bis.xml:815: Upon receipt of a <spanx style="verb">400 Bad >> Request</spanx> response, the client MUST NOT retry the request > > without >> >> modification. >> On 2014/05/16 10:42:38, Al Cutter wrote: >> > On 2014/05/16 10:15:13, Ben Laurie (Google) wrote: >> > > Isn't this just restating RFC 2616: >> > > >> > > "10.4.1 400 Bad Request >> > > >> > > The request could not be understood by the server due to > > malformed >> >> > > syntax. The client SHOULD NOT repeat the request without >> > > modifications." >> > >> > yes. > > >> My point was that if 2616 says it, why are we? And why this particular > > error >> >> code and not others? > > > Well, there's currently something talking to the prod logs which > definitely does not conform to the section of 2616 you pointed out so it > seemed worthwhile reinforcing it. We've also seen on numerous occasions > that the DigitalSignature has caused people confusion, so I have some > reason to doubt that people actually look at the referenced RFCs when > they're reading this one. > > If you feel strongly about it I'm happy to remove it though. If this is the reason its there, I do feel its redundant: who knows what someone will do wrong after the text is frozen? Should we quote the whole of RFC 2616? I think not. -- Certificate Transparency is hiring! Let me know if you're interested.
Sign in to reply to this message.
On 2014/05/19 13:26:56, Ben Laurie (Google) wrote: > On 19 May 2014 10:50, <mailto:alcutter@google.com> wrote: > > On 2014/05/16 10:50:18, Ben Laurie (Google) wrote: > >> > >> https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml > >> File doc/rfc6962-bis.xml (right): > > > > > > > > https://codereview.appspot.com/95310043/diff/1/doc/rfc6962-bis.xml#newcode815 > >> > >> doc/rfc6962-bis.xml:815: Upon receipt of a <spanx style="verb">400 Bad > >> Request</spanx> response, the client MUST NOT retry the request > > > > without > >> > >> modification. > >> On 2014/05/16 10:42:38, Al Cutter wrote: > >> > On 2014/05/16 10:15:13, Ben Laurie (Google) wrote: > >> > > Isn't this just restating RFC 2616: > >> > > > >> > > "10.4.1 400 Bad Request > >> > > > >> > > The request could not be understood by the server due to > > > > malformed > >> > >> > > syntax. The client SHOULD NOT repeat the request without > >> > > modifications." > >> > > >> > yes. > > > > > >> My point was that if 2616 says it, why are we? And why this particular > > > > error > >> > >> code and not others? > > > > > > Well, there's currently something talking to the prod logs which > > definitely does not conform to the section of 2616 you pointed out so it > > seemed worthwhile reinforcing it. We've also seen on numerous occasions > > that the DigitalSignature has caused people confusion, so I have some > > reason to doubt that people actually look at the referenced RFCs when > > they're reading this one. > > > > If you feel strongly about it I'm happy to remove it though. > > If this is the reason its there, I do feel its redundant: who knows > what someone will do wrong after the text is frozen? Should we quote > the whole of RFC 2616? I think not. > Dinged.
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
On 2014/05/19 14:32:03, Ben Laurie (Google) wrote: > LGTM Pushed as 2c718bf12b590d116e65552ec43cbdbf8656f1fe
Sign in to reply to this message.
|
