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

Fix sending headers in #send_P(int, PGM_P, PGM_P, size_t) #6881

Merged
merged 3 commits into from
Dec 10, 2019

Conversation

jennytoo
Copy link
Contributor

@jennytoo jennytoo commented Dec 5, 2019

The method #send(int, char*, char*[, size_t])) is a virtual method which
calculates the size of the content then calls #send_P(int, PGM_P, PGM_P,
size_t). This particular implementation of #send_P differs from the other
implementations of #send and #send_P in that it uses #sendContent for
headers and always calls #sendContent_P for contents even when the
contents is not specified.

The method #sendContent is intended for body and prepends the chunksize
in chunk mode but this breaks the HTTP protocol which does not expect a
chunksize prior to the headers.

Fix is simply to do the same thing as all the other methods - call
_currentClient.write and only call #sendContent_P if there is content to
send.

The method #send(int, char*, char*[, size_t])) is a virtual method which
calculates the size of the content then calls #send_P(int, PGM_P, PGM_P,
size_t). This particular implementation of #send_P differs from the other
implementations of #send and #send_P in that it uses #sendContent for
headers and always calls #sendContent_P for contents even when the
contents is not specified.

The method #sendContent is intended for body and prepends the chunksize
in chunk mode but this breaks the HTTP protocol which does not expect a
chunksize prior to the headers.

Fix is simply to do the same thing as all the other methods - call
_currentClient.write and only call #sendContent_P if there is content to
send.
@jennytoo
Copy link
Contributor Author

jennytoo commented Dec 5, 2019

I don't know if there was a reason this method differed from the other implementations of #send and #send_P, but it resulted in a regression in where send(200, "text/html", "") was intended to terminate headers and start content, but was now resulting in a header block with a chunk size prior to the headers, followed by a chunksize of 0 indicating end of body. Rewriting the call to send(200, "text/html", String("")) was sufficient to work around it because it forced usage of a different implementation of #send

@d-a-v
Copy link
Collaborator

d-a-v commented Dec 5, 2019

What is the previous working version you were using ?

@jennytoo
Copy link
Contributor Author

jennytoo commented Dec 6, 2019

I'll be honest, I don't know which was the previous working version. It was the one installed as part of Platform IO a couple months ago, and in debugging an issue in a newer installation I saw the ESP8266WebServer had been rewritten to use templates, and saw that the way it was being used resulted in calling the one #send / #send_P method which acted differently from the rest.

919c753 looks like the last good version with 36f9034 being the version which introduced the change.

The problem was not a change in #send_P, but the addition of a virtual #send method which matched the calling signature exactly (int, char*, char*) and then called #send_P. The actual code in this #send_P method has always worked this way it looks like - but it should never have worked in practice so either I'm misunderstanding it or no one ever relied on using this method while sending chunked data.

Copy link
Collaborator

@d-a-v d-a-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devyte devyte modified the milestones: 2.7.0, 2.6.3 Dec 10, 2019
@d-a-v d-a-v merged commit 70c3370 into esp8266:master Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants