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

Feature: Lf line break #64

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

suikabreaker
Copy link
Contributor

Fix #61

@suikabreaker
Copy link
Contributor Author

suikabreaker commented Jan 25, 2022

@zhuizhuhaomeng Also, can you review this PR? I will rebase later PR to the earlier accepted one.

@xiaocang
Copy link

According to rfc 7578 section 4.1, the boundary must:

constructed using CRLF, "--", and the value of the "boundary" parameter

It looks like it is not meeting the standard

@suikabreaker
Copy link
Contributor Author

suikabreaker commented Mar 23, 2022

According to rfc 7578 section 4.1, the boundary must:

constructed using CRLF, "--", and the value of the "boundary" parameter

It looks like it is not meeting the standard

I am aware of the RFC's requirements. But the fact is that Apache(mod_upload) and Nginx(upload module) (and maybe many other platforms) are compatible with requests using LF as line breaks, which may be a de facto standard compared to the RFC. To use OpenResty and resty.upload to act as a WAF filtering request body will be cheated by a malicious request that intentionally uses LF line breaks.

@xiaocang
Copy link

xiaocang commented Apr 7, 2022

@suikabreaker could you modify the code style by referring to another PR (#63) and rebase the latest master

t/sanity.t Show resolved Hide resolved
@suikabreaker
Copy link
Contributor Author

suikabreaker commented Apr 8, 2022

@suikabreaker could you modify the code style by referring to another PR (#63) and rebase the latest master

Not quite sure about the code style but I've checked for typos and added documentation.

@suikabreaker
Copy link
Contributor Author

The old bug shows again... Sorry but I don't have much time for it recently. Eventually I will fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support optional compatibility to LF as line break
3 participants