* [PATCH] Add some tolerance (RFC2616 sec. 19.3)
@ 2016-10-20 9:05 5% Mishael A Sibiryakov
2016-10-20 17:55 6% ` Eric Wong
0 siblings, 1 reply; 7+ results
From: Mishael A Sibiryakov @ 2016-10-20 9:05 UTC (permalink / raw)
To: unicorn-public
Hi all.
We're implementing client certificate authentication with nginx and
unicorn.
Nginx configured in the following way:
proxy_set_header X-SSL-Client-Cert $ssl_client_cert;
When client submits certificate and nginx passes it to the unicorn,
unicorn responds with 400 (Bad Request). This caused because nginx
doesn't use "\r\n" they using just "\n" and multilne headers is failed
to parse (I've added test).
Accorording to RFC2616 section 19.3:
https://www.w3.org/Protocols/rfc2616/rfc2616-sec19.html#sec19.3
"The line terminator for message-header fields is the sequence CRLF.
However, we recommend that applications, when parsing such headers,
recognize a single LF as a line terminator and ignore the leading CR."
CRLF changed to ("\r\n" | "\n")
Github commit https://github.com/uno4ki/unicorn/commit/ed127b66e162aaf1
76de05720f6be758f8b41b1f
PS: Googling "nginx unicorn ssl_client_cert" shows the problem.
---
ext/unicorn_http/unicorn_http_common.rl | 2 +-
test/unit/test_http_parser.rb | 16 ++++++++++++++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/ext/unicorn_http/unicorn_http_common.rl
b/ext/unicorn_http/unicorn_http_common.rl
index cc1d455..507e570 100644
--- a/ext/unicorn_http/unicorn_http_common.rl
+++ b/ext/unicorn_http/unicorn_http_common.rl
@@ -4,7 +4,7 @@
#### HTTP PROTOCOL GRAMMAR
# line endings
- CRLF = "\r\n";
+ CRLF = ("\r\n" | "\n");
# character types
CTL = (cntrl | 127);
diff --git a/test/unit/test_http_parser.rb
b/test/unit/test_http_parser.rb
index c72f7f2..4b1a16e 100644
--- a/test/unit/test_http_parser.rb
+++ b/test/unit/test_http_parser.rb
@@ -230,6 +230,22 @@ def test_nasty_pound_header
assert_equal expect, req['HTTP_X_SSL_BULLSHIT']
end
+ def test_multiline_header_0d0a
+ parser = HttpParser.new
+ parser.buf << "GET / HTTP/1.0\r\nX-Multiline-Header: foo
bar\r\n\tcha cha\r\n\tzha zha\r\n\r\n"
+ req = parser.env
+ assert_equal req, parser.parse
+ assert_equal 'foo bar cha cha zha zha',
req['HTTP_X_MULTILINE_HEADER']
+ end
+
+ def test_multiline_header_0a
+ parser = HttpParser.new
+ parser.buf << "GET / HTTP/1.0\nX-Multiline-Header: foo bar\n\tcha
cha\n\tzha zha\n\n"
+ req = parser.env
+ assert_equal req, parser.parse
+ assert_equal 'foo bar cha cha zha zha',
req['HTTP_X_MULTILINE_HEADER']
+ end
+
def test_continuation_eats_leading_spaces
parser = HttpParser.new
header = "GET / HTTP/1.1\r\n" \
--
2.10.1
^ permalink raw reply related [relevance 5%]
* Re: [PATCH] Add some tolerance (RFC2616 sec. 19.3)
2016-10-20 9:05 5% [PATCH] Add some tolerance (RFC2616 sec. 19.3) Mishael A Sibiryakov
@ 2016-10-20 17:55 6% ` Eric Wong
2016-10-20 20:25 7% ` Mishael A Sibiryakov
0 siblings, 1 reply; 7+ results
From: Eric Wong @ 2016-10-20 17:55 UTC (permalink / raw)
To: Mishael A Sibiryakov; +Cc: unicorn-public
Mishael A Sibiryakov <death@junki.org> wrote:
> Hi all.
>
> We're implementing client certificate authentication with nginx and
> unicorn.
>
> Nginx configured in the following way:
>
> proxy_set_header X-SSL-Client-Cert $ssl_client_cert;
>
> When client submits certificate and nginx passes it to the unicorn,
> unicorn responds with 400 (Bad Request). This caused because nginx
> doesn't use "\r\n" they using just "\n" and multilne headers is failed
> to parse (I've added test).
>
> Accorording to RFC2616 section 19.3:
> https://www.w3.org/Protocols/rfc2616/rfc2616-sec19.html#sec19.3
>
> "The line terminator for message-header fields is the sequence CRLF.
> However, we recommend that applications, when parsing such headers,
> recognize a single LF as a line terminator and ignore the leading CR."
>
> CRLF changed to ("\r\n" | "\n")
Thanks for that useful explanation. Aside from the unnecessary,
"Hi all,", that is an informative commit message which justifies
the usefulness of that patch.
> Github commit https://github.com/uno4ki/unicorn/commit/ed127b66e162aaf1
> 76de05720f6be758f8b41b1f
Unfortunately, the commit message in your git repo is lacking.
I've used the text at the top of your email.
> PS: Googling "nginx unicorn ssl_client_cert" shows the problem.
>
> ---
> ext/unicorn_http/unicorn_http_common.rl | 2 +-
> test/unit/test_http_parser.rb | 16 ++++++++++++++++
> 2 files changed, 17 insertions(+), 1 deletion(-)
Eeep, Evolution does some strange things with formatting
whitespaces. It looks like instructions for making it nicer are
in the Linux kernel:
https://bogomips.org/mirrors/linux.git/plain/Documentation/email-clients.txt?h=v4.8
<snip>
> + def test_multiline_header_0d0a
> + parser = HttpParser.new
> + parser.buf << "GET / HTTP/1.0\r\nX-Multiline-Header: foo
> bar\r\n\tcha cha\r\n\tzha zha\r\n\r\n"
I expect code to be wrapped at 80 lines or less. Fixed locally.
(I need big fonts, even 80 is a compromise, I really prefer 64)
Anyways, pushed to the "rfc2616-sec19.3" branch.
I've also uploaded a prerelease 5.1.0.4.gd5fbb to RubyGems
for folks without Ragel.
gem install --pre unicorn -v 5.1.0.4.gd5fbb
Anything else? Expect a 5.2.0 release in a few days or so.
Thanks.
^ permalink raw reply [relevance 6%]
* Re: [PATCH] Add some tolerance (RFC2616 sec. 19.3)
2016-10-20 17:55 6% ` Eric Wong
@ 2016-10-20 20:25 7% ` Mishael A Sibiryakov
2016-10-20 20:50 7% ` Eric Wong
0 siblings, 1 reply; 7+ results
From: Mishael A Sibiryakov @ 2016-10-20 20:25 UTC (permalink / raw)
To: Eric Wong; +Cc: unicorn-public
On Thu, 2016-10-20 at 17:55 +0000, Eric Wong wrote:
> Thanks for that useful explanation. Aside from the unnecessary,
> "Hi all,", that is an informative commit message which justifies
> the usefulness of that patch.
>
> >
> > Github commit https://github.com/uno4ki/unicorn/commit/ed127b66e162aaf1
> > 76de05720f6be758f8b41b1f
>
> Unfortunately, the commit message in your git repo is lacking.
> I've used the text at the top of your email.
It's just a temporary fork and I'm too lazy ))
> Eeep, Evolution does some strange things with formatting
> whitespaces. It looks like instructions for making it nicer are
> in the Linux kernel:
>
> https://bogomips.org/mirrors/linux.git/plain/Documentation/email-clients.txt?h=v4.8
Whoops, my fault.
> > + def test_multiline_header_0d0a
> > + parser = HttpParser.new
> > + parser.buf << "GET / HTTP/1.0\r\nX-Multiline-Header: foo
> > bar\r\n\tcha cha\r\n\tzha zha\r\n\r\n"
>
> I expect code to be wrapped at 80 lines or less. Fixed locally.
> (I need big fonts, even 80 is a compromise, I really prefer 64)
Line 221 with "ssl bullshit" guided me to this.
> Anyways, pushed to the "rfc2616-sec19.3" branch.
>
> I've also uploaded a prerelease 5.1.0.4.gd5fbb to RubyGems
> for folks without Ragel.
>
> gem install --pre unicorn -v 5.1.0.4.gd5fbb
>
> Anything else? Expect a 5.2.0 release in a few days or so.
> Thanks.
Thanks for the quick acceptance.
^ permalink raw reply [relevance 7%]
* Re: [PATCH] Add some tolerance (RFC2616 sec. 19.3)
2016-10-20 20:25 7% ` Mishael A Sibiryakov
@ 2016-10-20 20:50 7% ` Eric Wong
2016-10-20 21:03 7% ` Mishael A Sibiryakov
0 siblings, 1 reply; 7+ results
From: Eric Wong @ 2016-10-20 20:50 UTC (permalink / raw)
To: Mishael A Sibiryakov; +Cc: unicorn-public
Mishael A Sibiryakov <death@junki.org> wrote:
> It's just a temporary fork and I'm too lazy ))
<snip>
> Whoops, my fault.
No worries :)
> > > + def test_multiline_header_0d0a
> > > + parser = HttpParser.new
> > > + parser.buf << "GET / HTTP/1.0\r\nX-Multiline-Header: foo
> > > bar\r\n\tcha cha\r\n\tzha zha\r\n\r\n"
> >
> > I expect code to be wrapped at 80 lines or less. Fixed locally.
> > (I need big fonts, even 80 is a compromise, I really prefer 64)
>
> Line 221 with "ssl bullshit" guided me to this.
Ah, that's an old test from Mongrel, probably not worth fixing;
just trying to keep new code cleaner.
^ permalink raw reply [relevance 7%]
* Re: [PATCH] Add some tolerance (RFC2616 sec. 19.3)
2016-10-20 20:50 7% ` Eric Wong
@ 2016-10-20 21:03 7% ` Mishael A Sibiryakov
0 siblings, 0 replies; 7+ results
From: Mishael A Sibiryakov @ 2016-10-20 21:03 UTC (permalink / raw)
To: Eric Wong; +Cc: unicorn-public
On Thu, 2016-10-20 at 20:50 +0000, Eric Wong wrote:
> > > > + def test_multiline_header_0d0a
> > > > + parser = HttpParser.new
> > > > + parser.buf << "GET / HTTP/1.0\r\nX-Multiline-Header: foo
> > > > bar\r\n\tcha cha\r\n\tzha zha\r\n\r\n"
> > >
> > > I expect code to be wrapped at 80 lines or less. Fixed locally.
> > > (I need big fonts, even 80 is a compromise, I really prefer 64)
> >
> > Line 221 with "ssl bullshit" guided me to this.
>
> Ah, that's an old test from Mongrel, probably not worth fixing;
> just trying to keep new code cleaner.
It's a good desire :) I'll comply next time.
^ permalink raw reply [relevance 7%]
* [ANN] unicorn 5.2.0 - Rack HTTP server for fast clients and *nix
[not found] <20161031-unicorn-5.2.0-released@bogomips.org>
@ 2016-10-31 20:04 5% ` Eric Wong
0 siblings, 0 replies; 7+ results
From: Eric Wong @ 2016-10-31 20:04 UTC (permalink / raw)
To: ruby-talk, unicorn-public; +Cc: Mishael A Sibiryakov
unicorn is an HTTP server for Rack applications designed to only serve
fast clients on low-latency, high-bandwidth connections and take
advantage of features in Unix/Unix-like kernels. Slow clients should
only be served by placing a reverse proxy capable of fully buffering
both the the request and response in between unicorn and slow clients.
* https://bogomips.org/unicorn/
* public list: unicorn-public@bogomips.org
* mail archives: https://bogomips.org/unicorn-public/
* git clone git://bogomips.org/unicorn.git
* https://bogomips.org/unicorn/NEWS.atom.xml
* nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn
Changes:
Most notably, this release allows us to support requests with
lines delimited by LF-only, as opposed to the standard CRLF
pair and allowed by RFC 2616 sec 19.3.
Thanks to Mishael A Sibiryakov for the explanation and change:
https://bogomips.org/unicorn-public/1476954332.1736.156.camel@junki.org/
Thanks to Let's Encrypt, the website also moves to HTTPS
<https://bogomips.org/unicorn/> to improve reader privacy. The
"unicorn.bogomips.org" subdomain will be retired soon to reduce
subjectAltName bloat and speed up certificate renewals.
There's also the usual round of documentation and example
updates, too.
Eric Wong (7):
examples/init.sh: update to reduce upgrade raciness
doc: systemd should only kill master in example
examples/logrotate.conf: update example for systemd
doc: update gmane URLs to point to our own archives
relocate website to https://bogomips.org/unicorn/
TODO: remove Rack 2.x item
build: "install-gem" target avoids network
Mishael A Sibiryakov (1):
Add some tolerance (RFC2616 sec. 19.3)
^ permalink raw reply [relevance 5%]
* [PATCH] unicorn_http_common.rl: use only ASCII spaces for compatibility
@ 2023-06-20 10:46 5% EW
0 siblings, 0 replies; 7+ results
From: EW @ 2023-06-20 10:46 UTC (permalink / raw)
To: unicorn-public
Ragel 6.10 on FreeBSD 12.4 amd64 complains and fails on this, yet the same
Ragel version on Debian 11.x i386 and amd64 never has. I suspect this can
fix compatibility on s390x, arm64, armel, and armhf Debian builds:
https://buildd.debian.org/status/fetch.php?pkg=unicorn&arch=s390x&ver=6.1.0-1&stamp=1687156375&file=log
https://buildd.debian.org/status/fetch.php?pkg=unicorn&arch=arm64&ver=6.1.0-1&stamp=1687156478&file=log
https://buildd.debian.org/status/fetch.php?pkg=unicorn&arch=armel&ver=6.1.0-1&stamp=1687156619&file=log
https://buildd.debian.org/status/fetch.php?pkg=unicorn&arch=armhf&ver=6.1.0-1&stamp=1687156807&file=log
Fixes: d5fbbf547203061b (Add some tolerance (RFC2616 sec. 19.3), 2016-10-20)
---
ext/unicorn_http/unicorn_http_common.rl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ext/unicorn_http/unicorn_http_common.rl b/ext/unicorn_http/unicorn_http_common.rl
index 0988b54..507e570 100644
--- a/ext/unicorn_http/unicorn_http_common.rl
+++ b/ext/unicorn_http/unicorn_http_common.rl
@@ -4,7 +4,7 @@
#### HTTP PROTOCOL GRAMMAR
# line endings
- CRLF = ("\r\n" | "\n");
+ CRLF = ("\r\n" | "\n");
# character types
CTL = (cntrl | 127);
^ permalink raw reply related [relevance 5%]
Results 1-7 of 7 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2016-10-20 9:05 5% [PATCH] Add some tolerance (RFC2616 sec. 19.3) Mishael A Sibiryakov
2016-10-20 17:55 6% ` Eric Wong
2016-10-20 20:25 7% ` Mishael A Sibiryakov
2016-10-20 20:50 7% ` Eric Wong
2016-10-20 21:03 7% ` Mishael A Sibiryakov
[not found] <20161031-unicorn-5.2.0-released@bogomips.org>
2016-10-31 20:04 5% ` [ANN] unicorn 5.2.0 - Rack HTTP server for fast clients and *nix Eric Wong
2023-06-20 10:46 5% [PATCH] unicorn_http_common.rl: use only ASCII spaces for compatibility EW
Code repositories for project(s) associated with this public inbox
https://yhbt.net/unicorn.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).