Skip to content

/ go

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

x/net/http2: WriteDataPadded shouldn't accepted padding characters #18809

Closed
daurnimator opened this issue Jan 27, 2017 · 2 comments
Closed

x/net/http2: WriteDataPadded shouldn't accepted padding characters #18809

daurnimator opened this issue Jan 27, 2017 · 2 comments
Assignees
@bradfitz
Labels
FrozenDueToAge
Milestone
Go1.9

Comments

@daurnimator
Copy link

@daurnimator daurnimator commented Jan 27, 2017

Carrying over from summerwind/h2spec#71 (comment)

In http2, padding MUST be null bytes. It makes no sense for WriteDataPadded to take padding bytes as an argument (it should only take the number of bytes)
@daurnimator daurnimator changed the title x/net/http2: WriteDataPadded should taking padding characters x/net/http2: WriteDataPadded shouldn't taking padding characters Jan 27, 2017
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jan 27, 2017

Note the API permits writing invalid stuff:

https://godoc.org/golang.org/x/net/http2#Framer

... has both AllowIllegalWrites and AllowIllegalReads.

This is useful for testing other implementations.

But I agree that there's something to do here. We should probably validate the pad on write unless AllowIllegalWrites is set.
@bradfitz bradfitz self-assigned this Jan 27, 2017
@odeke-em odeke-em changed the title x/net/http2: WriteDataPadded shouldn't taking padding characters x/net/http2: WriteDataPadded shouldn't accepted padding characters Jan 27, 2017
@bradfitz bradfitz added this to the Go1.9 milestone Feb 1, 2017
@gopherbot
Copy link

@gopherbot gopherbot commented Feb 1, 2017

CL https://golang.org/cl/36118 mentions this issue.
@golang golang locked and limited conversation to collaborators Feb 1, 2018
c3mb0 pushed a commit to c3mb0/net that referenced this issue Apr 2, 2018
@bradfitz
http2: don't allow writing non-zero padding unless Framer.AllowIllega…
6ed8880 
…lWrites

Fixes golang/go#18809

Change-Id: Ib1014f3ebe5a57dde30b4eaf287a2cbff3c1179c
Reviewed-on: https://go-review.googlesource.com/36118
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Tom Bergan <tombergan@google.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bradfitz bradfitz

Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.9
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
@bradfitz @daurnimator @gopherbot