git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC 0/1] mailinfo: de-quote quoted-pair in header fields

From
Kevin Daudt <me@ikke.info>
Date
Sep 14, 2016, 04:58 UTC
Message-ID
<20160914045841.GA26893@ikke.info>
In-Reply-To
<xmqqr38ns5wi.fsf@gitster.mtv.corp.google.com>
On Tue, Sep 13, 2016 at 05:04:45PM -0700, Junio C Hamano wrote:
Show 7 quoted lines
> Kevin Daudt <me@ikke.info> writes:
> 
> 
> It often is easier to read if smaller of the two are in the if part
> and the larger in else part.  Also your switch/case is indented one
> level too deep.  I.e.
> 
Thanks, I've switched the order and fixed indentation.
Show 7 quoted lines
> 
> I found the variable name "skip" a bit hard to reason about.  What
> it does is to signal the next round of the processing that we have
> seen a single-byte quote and it should keep the byte it will get, no
> matter what its value is.  It is "skipping" the conditional
> processing, but I'd imagine most people would consider it is
> "keeping the byte".

Yes, I agree and was trying to find a better name. I have renamed it to "take_next_literally", which indicates better what it means.

Show 18 quoted lines
> 
> > @@ -461,6 +506,7 @@ static int check_header(struct mailinfo *mi,
> >  			 */
> >  			strbuf_add(&sb, line->buf + len + 2, line->len - len - 2);
> >  			decode_header(mi, &sb);
> > +			unescape_quoted_pair(mi, &sb);
> >  			handle_header(&hdr_data[i], &sb);
> >  			ret = 1;
> >  			goto check_header_out;
> 
> I wonder why this call is only in here, not on other headers that
> all call decode_header().  For that matter, I wonder if the call (or
> the logic of the helper function itself) should go at the end of
> decode_header().  After all, this is different kind of decoding; the
> current one knows how to do b/q encoding but forgot about the more
> traditional quoting done with backslash, and you are teaching the
> code that the current decoding it does is insufficient and how to
> handle the one that the original implementors forgot about.

Makes sense, it should be applied to all headers (I missed the other decode_header calls).

I will send a new version later.
Previous: Junio C HamanoNext: Jeff King
Message 6 of 16 in “git-am includes escape characters from 'From' field”
  1. Swift GeekSep 12, 2016
  2. Jeff KingSep 13, 2016
  3. 0/1 de-quote quoted-strings in mailinfoKevin Daudt, Sep 13, 2016
  4. 0/1 mailinfo: de-quote quoted-pair in header fieldsKevin Daudt, Sep 13, 2016
  5. Junio C HamanoSep 14, 2016
  6. Kevin DaudtSep 14, 2016
  7. Jeff KingSep 14, 2016
  8. Junio C HamanoSep 14, 2016
  9. Kevin DaudtSep 14, 2016
  10. Junio C HamanoSep 14, 2016
  11. Jeff KingSep 14, 2016
  12. Junio C HamanoSep 14, 2016
  13. Jeff KingSep 14, 2016
  14. Kevin DaudtSep 15, 2016
  15. Jeff KingSep 15, 2016
  16. Jeff KingSep 14, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.