Skip to content
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

Jump to bottom

mime: BEncoding and QEncoding don't respect the 76 character line limit in RFC2047 #19617

Open
joegrasse opened this issue Mar 20, 2017 · 15 comments
Open

mime: BEncoding and QEncoding don't respect the 76 character line limit in RFC2047 #19617

joegrasse opened this issue Mar 20, 2017 · 15 comments
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unplanned

Comments

@joegrasse
Copy link

joegrasse commented Mar 20, 2017 •
edited

As a continuation of Issue #12300, mime: BEncoding and QEncoding don't respect the 76 character line limit in RFC2047.

Here is the relevant part from Section 2 of RFC2047. (emphasis mine)

An 'encoded-word' may not be more than 75 characters long, including
'charset', 'encoding', 'encoded-text', and delimiters. If it is
desirable to encode more text than will fit in an 'encoded-word' of
75 characters, multiple 'encoded-word's (separated by CRLF SPACE) may
be used.

While there is no limit to the length of a multiple-line header
field, each line of a header field that contains one or more
'encoded-word's is limited to 76 characters.

What version of Go are you using (go version)?

1.8

What operating system and processor architecture are you using (go env)?

set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=E:\gowork
set GORACE=
set GOROOT=C:\Go
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\graj04\AppData\Local\Temp\go-build930053034=/tmp/go-build -gno-record-gcc-switches
set CXX=g++
set CGO_ENABLED=1

What did you do?

https://play.golang.org/p/vO6pcZgesh

What did you expect to see?

=?utf-8?q?=C2=A1Hola,_se=C3=B1or_this_is_to_see_what_happens_if_the_test_?= 
 =?utf-8?q?is_really_long._ltkhetslk_lskthe_lt_slkth_sekltse_ltksht_lskejt?= 
 =?utf-8?q?_lsht_lsht_lht_lht_lsht_lhtlshtlsetlksje_tlsejt_lksetlkse_tlkse?= 
 =?utf-8?q?_tlkst_lkstlkst_lskht_lstlsht_lshtlshtlshtslhtlshtlstle_tlse_tl?= 
 =?utf-8?q?se_tlkse_tlkse_tlse_tlst_lsetlstlset_setsle_tlset_lst_lset_lskt?= 
 =?utf-8?q?_alkthalkht_alkth_aeklth_aeslth_aeslth_eajraoiflanvwoalnwoaiehf?= 
 =?utf-8?q?_awesi_leahr_aflhf_awelh_aweh_awelh_wahwe_!?=

What did you see instead?

=?utf-8?q?=C2=A1Hola,_se=C3=B1or_this_is_to_see_what_happens_if_the_test_?= =?utf-8?q?is_really_long._ltkhetslk_lskthe_lt_slkth_sekltse_ltksht_lskejt?= =?utf-8?q?_lsht_lsht_lht_lht_lsht_lhtlshtlsetlksje_tlsejt_lksetlkse_tlkse?= =?utf-8?q?_tlkst_lkstlkst_lskht_lstlsht_lshtlshtlshtslhtlshtlstle_tlse_tl?= =?utf-8?q?se_tlkse_tlkse_tlse_tlst_lsetlstlset_setsle_tlset_lst_lset_lskt?= =?utf-8?q?_alkthalkht_alkth_aeklth_aeslth_aeslth_eajraoiflanvwoalnwoaiehf?= =?utf-8?q?_awesi_leahr_aflhf_awelh_aweh_awelh_wahwe_!?=
@bradfitz bradfitz added this to the Go1.9Maybe milestone Mar 21, 2017
@bradfitz bradfitz added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Mar 21, 2017
@joegrasse
Copy link
Author

Looking at this a little more, it looks like it might just be changing this line from buf.WriteByte(' ') to buf.WriteString("\r\n ")

@bradfitz
Copy link
Contributor

Oh, in that case I think this is working as intended. I don't think we want to start returning newlines here. We already broke it up into chunks so another layer can wrap it on the whitespace when putting it into a format requiring short lines.

@bradfitz
Copy link
Contributor

@alexcesaro, thoughts?

@bradfitz bradfitz added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 22, 2017
@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@joegrasse
Copy link
Author

This should be reopened. It is waiting on developer feedback.

@joegrasse
Copy link
Author

Cc @alexcesaro @bradfitz

@bradfitz
Copy link
Contributor

Reopening, but I don't think we can change the behavior here.

We could document it, and/or we could add a new method to do the newline behavior perhaps.

@bradfitz bradfitz reopened this Apr 24, 2017
@bradfitz bradfitz removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Apr 24, 2017
@bradfitz bradfitz modified the milestones: Unplanned, Go1.9Maybe Apr 24, 2017
@joegrasse
Copy link
Author

@bradfitz As far as what these encoders should do, I would think it would depend on if they are trying to implement RFC2047 or just be a QEncoder and BEncoder. I would think if they are just being a Q/BEncoder then they wouldn't add the spaces to break the encoded text into chunks and add "=?utf-8?q?=", etc. If they are trying to implement RFC2047, then they should break into chunks, line wrap, etc. Right now I think it is straddling the line.

@joegrasse
Copy link
Author

@bradfitz It looks like @alexcesaro may not be responding. Is this waiting on him or can someone else provide feedback on my previous comment?

@joegrasse
Copy link
Author

@bradfitz Just trying to move this somewhere again. Any response to my previous two comments?

@bradfitz
Copy link
Contributor

I no longer have RFC2047 and [QB]Encoder specs in my head, sorry.

But I don't think this package should be in the business of adding line breaks. That seems like the job of something write headers. Even today if you try to write a header with newlines, the http package for instance will scrub your newlines (https://play.golang.org/p/vBkBjEU3M4H). I'm not sure anything else even writes them in the standard library? net/textproto only reads them it looks like.

If we cared about where newlines go, we need a design for what layer inserts them and why, and maybe it's just automatic at 76, in which case the mime package needs no changes.

@joegrasse
Copy link
Author

and maybe it's just automatic at 76, in which case the mime package needs no changes.

I'm not quite clear on this part. This bug is about the fact that the [QB]Encoder doesn't insert newlines at 76 characters.

@bradfitz
Copy link
Contributor

This bug is about the fact that the [QB]Encoder doesn't insert newlines at 76 characters.

Yeah, I understand. And I'm proposing that perhaps it should not, because adding newlines should be done at a different layer. We need to think about it.

@joegrasse
Copy link
Author

When you say:

We need to think about it.

Do you mean the Go developers?

@bradfitz
Copy link
Contributor

We are all Go developers. Anybody needs to think about it and propose a plan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@bradfitz @alexcesaro @joegrasse @gopherbot