[jetty-dev] Possible race condition with ByteBuffers in HttpParser

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[jetty-dev] Possible race condition with ByteBuffers in HttpParser

Daniel Potter

Hello jetty-dev,

 

I’ve been debugging a rather strange, timing-sensitive bug in a project that showed up after upgrading jetty from 9.2.13 -> 9.4.8 (still repro on 9.4.11), and have stumbled upon some suspicious behavior within jetty’s request handling/HTTP parsing layers. I haven’t been able to isolate a standalone repro case yet, but while I work on doing so, I was wondering if any of this sounded familiar or suspicious to anyone?

 

Summary of conditions to repro:

  • High volume of GET requests (200-400 per second)
  • A fairly small payload per request (around 500 bytes including all HTTP headers)

 

Observed repro:

  • Somewhat sporadically (usually between 5-10 minutes, although it can take up to 30), we see an HTTP 400 error returned to the client, due to “unknown version” (i.e., invalid HTTP version spec in the request). However, there is nothing unusual about the request that fails (compared to all the thousands that succeed). I’ve confirmed via Wireshark that the exact request in question is perfectly valid.

 

  • With similar probability, we sometimes see a duplicated response from an earlier request sent as the response to the subsequent request (so far, always on the same connection, but we haven’t ruled out that it might be happening across multiple connections). The end result is that all HTTP responses are shifted down by one, causing mayhem on the client side. According to Wireshark, the HTTP stream goes something like this:

    Request 1
    Response 1

    Request 2
    Response 2

    Request 3
    Response 2

    Request 4
    Response 3

    Request 5
    Response 4

 

I’ve narrowed the investigation down to what appears to be a race condition in filling on-heap buffers from an org.eclipse.jetty.io.ByteBufferPool with the HTTP request bytes, and parsing the request out of those same buffers. In particular, when the 400 “unknown version” error hits, org.eclipse.jetty.http.HttpParser.parseNext(buffer) is called with a buffer that appears to be in fill mode (position at end-of-content, limit at max capacity); yet by the time code has progressed to where the BAD_REQUEST_400 is thrown, the buffer has flipped back to flush mode (with position and limit both at the end-of-content). _methodString and _uri are both parsed to be gibberish (based on content that is read past the end of the buffer’s limit), and _version remains null.

 

Can anyone help confirm that the ByteBuffer behavior inside of HttpParser is unexpected? And, if by chance, this looks like anything else you all may have encountered?

 

Regards,
Daniel

 

Daniel Potter // Senior Software Engineer
PROS® // Powering Modern Commerce with Dynamic Pricing Science

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev
Reply | Threaded
Open this post in threaded view
|

Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

Joakim Erdfelt-8
Please upgrade to Jetty 9.4.11.v20180605

There's been a tremendous amount of updates and fixes for RFC7230 since the 9.4.8 days.
Including a long raft of improvements for legacy RFC2616 modes (where most of your issues exist), now available behind configurable Compliance modes.


Joakim Erdfelt / [hidden email]

On Tue, Jun 19, 2018 at 2:51 PM, Daniel Potter <[hidden email]> wrote:

Hello jetty-dev,

 

I’ve been debugging a rather strange, timing-sensitive bug in a project that showed up after upgrading jetty from 9.2.13 -> 9.4.8 (still repro on 9.4.11), and have stumbled upon some suspicious behavior within jetty’s request handling/HTTP parsing layers. I haven’t been able to isolate a standalone repro case yet, but while I work on doing so, I was wondering if any of this sounded familiar or suspicious to anyone?

 

Summary of conditions to repro:

  • High volume of GET requests (200-400 per second)
  • A fairly small payload per request (around 500 bytes including all HTTP headers)

 

Observed repro:

  • Somewhat sporadically (usually between 5-10 minutes, although it can take up to 30), we see an HTTP 400 error returned to the client, due to “unknown version” (i.e., invalid HTTP version spec in the request). However, there is nothing unusual about the request that fails (compared to all the thousands that succeed). I’ve confirmed via Wireshark that the exact request in question is perfectly valid.

 

  • With similar probability, we sometimes see a duplicated response from an earlier request sent as the response to the subsequent request (so far, always on the same connection, but we haven’t ruled out that it might be happening across multiple connections). The end result is that all HTTP responses are shifted down by one, causing mayhem on the client side. According to Wireshark, the HTTP stream goes something like this:

    Request 1
    Response 1

    Request 2
    Response 2

    Request 3
    Response 2

    Request 4
    Response 3

    Request 5
    Response 4

 

I’ve narrowed the investigation down to what appears to be a race condition in filling on-heap buffers from an org.eclipse.jetty.io.ByteBufferPool with the HTTP request bytes, and parsing the request out of those same buffers. In particular, when the 400 “unknown version” error hits, org.eclipse.jetty.http.HttpParser.parseNext(buffer) is called with a buffer that appears to be in fill mode (position at end-of-content, limit at max capacity); yet by the time code has progressed to where the BAD_REQUEST_400 is thrown, the buffer has flipped back to flush mode (with position and limit both at the end-of-content). _methodString and _uri are both parsed to be gibberish (based on content that is read past the end of the buffer’s limit), and _version remains null.

 

Can anyone help confirm that the ByteBuffer behavior inside of HttpParser is unexpected? And, if by chance, this looks like anything else you all may have encountered?

 

Regards,
Daniel

 

Daniel Potter // Senior Software Engineer
PROS® // Powering Modern Commerce with Dynamic Pricing Science

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev
Reply | Threaded
Open this post in threaded view
|

Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

Joakim Erdfelt-8
In reply to this post by Daniel Potter
Also, what kind of client are you using to test with?
The description smells of a protocol bug introduced by a client. (the "unknown version" portion tells us the parse was parsing a line that didn't have a HTTP version)

If you have a setup that even occasionally produces this result we would like to see it.


Joakim Erdfelt / [hidden email]

On Tue, Jun 19, 2018 at 2:51 PM, Daniel Potter <[hidden email]> wrote:

Hello jetty-dev,

 

I’ve been debugging a rather strange, timing-sensitive bug in a project that showed up after upgrading jetty from 9.2.13 -> 9.4.8 (still repro on 9.4.11), and have stumbled upon some suspicious behavior within jetty’s request handling/HTTP parsing layers. I haven’t been able to isolate a standalone repro case yet, but while I work on doing so, I was wondering if any of this sounded familiar or suspicious to anyone?

 

Summary of conditions to repro:

  • High volume of GET requests (200-400 per second)
  • A fairly small payload per request (around 500 bytes including all HTTP headers)

 

Observed repro:

  • Somewhat sporadically (usually between 5-10 minutes, although it can take up to 30), we see an HTTP 400 error returned to the client, due to “unknown version” (i.e., invalid HTTP version spec in the request). However, there is nothing unusual about the request that fails (compared to all the thousands that succeed). I’ve confirmed via Wireshark that the exact request in question is perfectly valid.

 

  • With similar probability, we sometimes see a duplicated response from an earlier request sent as the response to the subsequent request (so far, always on the same connection, but we haven’t ruled out that it might be happening across multiple connections). The end result is that all HTTP responses are shifted down by one, causing mayhem on the client side. According to Wireshark, the HTTP stream goes something like this:

    Request 1
    Response 1

    Request 2
    Response 2

    Request 3
    Response 2

    Request 4
    Response 3

    Request 5
    Response 4

 

I’ve narrowed the investigation down to what appears to be a race condition in filling on-heap buffers from an org.eclipse.jetty.io.ByteBufferPool with the HTTP request bytes, and parsing the request out of those same buffers. In particular, when the 400 “unknown version” error hits, org.eclipse.jetty.http.HttpParser.parseNext(buffer) is called with a buffer that appears to be in fill mode (position at end-of-content, limit at max capacity); yet by the time code has progressed to where the BAD_REQUEST_400 is thrown, the buffer has flipped back to flush mode (with position and limit both at the end-of-content). _methodString and _uri are both parsed to be gibberish (based on content that is read past the end of the buffer’s limit), and _version remains null.

 

Can anyone help confirm that the ByteBuffer behavior inside of HttpParser is unexpected? And, if by chance, this looks like anything else you all may have encountered?

 

Regards,
Daniel

 

Daniel Potter // Senior Software Engineer
PROS® // Powering Modern Commerce with Dynamic Pricing Science

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev
Reply | Threaded
Open this post in threaded view
|

Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

Daniel Potter

Hi Joakim,

 

Certainly, we’ll upgrade to 9.4.11 ASAP… but the issue is still present, unfortunately. Apologies for the outdated links, I’ve been chasing this bug for a while.

 

I was suspicious of the client at first, too (jersey 2.26), but the request doesn’t look wrong at all to me. Here’s an example of the snippet extracted by Wireshark (Auth token change is my edit, but this should still give a more clear example)—request first, followed by the response from jetty:

 

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1

Authorization: Bearer tokenWhichIsBase64

debug-request-id: ITEM-8fe57f6fcc81573a

User-Agent: Jersey/2.26 (HttpUrlConnection 1.8.0_172)

Host: localhost:18443

Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2

Connection: keep-alive

 

HTTP/1.1 400 Unknown Version

Content-Type: text/html;charset=iso-8859-1

Content-Length: 58

Connection: close

 

<h1>Bad Message 400</h1><pre>reason: Unknown Version</pre>

 

I’ll upgrade my local build to 9.4.11 and will keep trying to provide an isolated repro case.

 

Best,
Daniel

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 15:26
To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Also, what kind of client are you using to test with?

The description smells of a protocol bug introduced by a client. (the "unknown version" portion tells us the parse was parsing a line that didn't have a HTTP version)

 

If you have a setup that even occasionally produces this result we would like to see it.

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 2:51 PM, Daniel Potter <[hidden email]> wrote:

Hello jetty-dev,

 

I’ve been debugging a rather strange, timing-sensitive bug in a project that showed up after upgrading jetty from 9.2.13 -> 9.4.8 (still repro on 9.4.11), and have stumbled upon some suspicious behavior within jetty’s request handling/HTTP parsing layers. I haven’t been able to isolate a standalone repro case yet, but while I work on doing so, I was wondering if any of this sounded familiar or suspicious to anyone?

 

Summary of conditions to repro:

  • High volume of GET requests (200-400 per second)
  • A fairly small payload per request (around 500 bytes including all HTTP headers)

 

Observed repro:

  • Somewhat sporadically (usually between 5-10 minutes, although it can take up to 30), we see an HTTP 400 error returned to the client, due to “unknown version” (i.e., invalid HTTP version spec in the request). However, there is nothing unusual about the request that fails (compared to all the thousands that succeed). I’ve confirmed via Wireshark that the exact request in question is perfectly valid.

 

  • With similar probability, we sometimes see a duplicated response from an earlier request sent as the response to the subsequent request (so far, always on the same connection, but we haven’t ruled out that it might be happening across multiple connections). The end result is that all HTTP responses are shifted down by one, causing mayhem on the client side. According to Wireshark, the HTTP stream goes something like this:

    Request 1
    Response 1

    Request 2
    Response 2

    Request 3
    Response 2

    Request 4
    Response 3

    Request 5
    Response 4

 

I’ve narrowed the investigation down to what appears to be a race condition in filling on-heap buffers from an org.eclipse.jetty.io.ByteBufferPool with the HTTP request bytes, and parsing the request out of those same buffers. In particular, when the 400 “unknown version” error hits, org.eclipse.jetty.http.HttpParser.parseNext(buffer) is called with a buffer that appears to be in fill mode (position at end-of-content, limit at max capacity); yet by the time code has progressed to where the BAD_REQUEST_400 is thrown, the buffer has flipped back to flush mode (with position and limit both at the end-of-content). _methodString and _uri are both parsed to be gibberish (based on content that is read past the end of the buffer’s limit), and _version remains null.

 

Can anyone help confirm that the ByteBuffer behavior inside of HttpParser is unexpected? And, if by chance, this looks like anything else you all may have encountered?

 

Regards,
Daniel

 

Daniel Potter // Senior Software Engineer
PROS® // Powering Modern Commerce with Dynamic Pricing Science

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev
Reply | Threaded
Open this post in threaded view
|

Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

Joakim Erdfelt-8
Strange request ...

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1
...
Connection: keep-alive

Your client specified "HTTP/1.1" and then used "Connection: keep-alive" (a HTTP/1.0 only feature/concept).

Is this over SSL/TLS perhaps?
If so, does this issue manifest without SSL/TLS?


Joakim Erdfelt / [hidden email]

On Tue, Jun 19, 2018 at 4:04 PM, Daniel Potter <[hidden email]> wrote:

Hi Joakim,

 

Certainly, we’ll upgrade to 9.4.11 ASAP… but the issue is still present, unfortunately. Apologies for the outdated links, I’ve been chasing this bug for a while.

 

I was suspicious of the client at first, too (jersey 2.26), but the request doesn’t look wrong at all to me. Here’s an example of the snippet extracted by Wireshark (Auth token change is my edit, but this should still give a more clear example)—request first, followed by the response from jetty:

 

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1

Authorization: Bearer tokenWhichIsBase64

debug-request-id: ITEM-8fe57f6fcc81573a

User-Agent: Jersey/2.26 (HttpUrlConnection 1.8.0_172)

Host: localhost:18443

Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2

Connection: keep-alive

 

HTTP/1.1 400 Unknown Version

Content-Type: text/html;charset=iso-8859-1

Content-Length: 58

Connection: close

 

<h1>Bad Message 400</h1><pre>reason: Unknown Version</pre>

 

I’ll upgrade my local build to 9.4.11 and will keep trying to provide an isolated repro case.

 

Best,
Daniel

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 15:26
To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Also, what kind of client are you using to test with?

The description smells of a protocol bug introduced by a client. (the "unknown version" portion tells us the parse was parsing a line that didn't have a HTTP version)

 

If you have a setup that even occasionally produces this result we would like to see it.

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 2:51 PM, Daniel Potter <[hidden email]> wrote:

Hello jetty-dev,

 

I’ve been debugging a rather strange, timing-sensitive bug in a project that showed up after upgrading jetty from 9.2.13 -> 9.4.8 (still repro on 9.4.11), and have stumbled upon some suspicious behavior within jetty’s request handling/HTTP parsing layers. I haven’t been able to isolate a standalone repro case yet, but while I work on doing so, I was wondering if any of this sounded familiar or suspicious to anyone?

 

Summary of conditions to repro:

  • High volume of GET requests (200-400 per second)
  • A fairly small payload per request (around 500 bytes including all HTTP headers)

 

Observed repro:

  • Somewhat sporadically (usually between 5-10 minutes, although it can take up to 30), we see an HTTP 400 error returned to the client, due to “unknown version” (i.e., invalid HTTP version spec in the request). However, there is nothing unusual about the request that fails (compared to all the thousands that succeed). I’ve confirmed via Wireshark that the exact request in question is perfectly valid.

 

  • With similar probability, we sometimes see a duplicated response from an earlier request sent as the response to the subsequent request (so far, always on the same connection, but we haven’t ruled out that it might be happening across multiple connections). The end result is that all HTTP responses are shifted down by one, causing mayhem on the client side. According to Wireshark, the HTTP stream goes something like this:

    Request 1
    Response 1

    Request 2
    Response 2

    Request 3
    Response 2

    Request 4
    Response 3

    Request 5
    Response 4

 

I’ve narrowed the investigation down to what appears to be a race condition in filling on-heap buffers from an org.eclipse.jetty.io.ByteBufferPool with the HTTP request bytes, and parsing the request out of those same buffers. In particular, when the 400 “unknown version” error hits, org.eclipse.jetty.http.HttpParser.parseNext(buffer) is called with a buffer that appears to be in fill mode (position at end-of-content, limit at max capacity); yet by the time code has progressed to where the BAD_REQUEST_400 is thrown, the buffer has flipped back to flush mode (with position and limit both at the end-of-content). _methodString and _uri are both parsed to be gibberish (based on content that is read past the end of the buffer’s limit), and _version remains null.

 

Can anyone help confirm that the ByteBuffer behavior inside of HttpParser is unexpected? And, if by chance, this looks like anything else you all may have encountered?

 

Regards,
Daniel

 

Daniel Potter // Senior Software Engineer
PROS® // Powering Modern Commerce with Dynamic Pricing Science

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev
Reply | Threaded
Open this post in threaded view
|

Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

Daniel Potter

Agreed—that’s definitely the client’s quirk. Still, it should be ignored and harmless when using HTTP/1.1 to spec, I believe?

 

I confirmed the bug both with, and without, TLS.

 

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 16:11
To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Strange request ...

 

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1

...

Connection: keep-alive

 

Your client specified "HTTP/1.1" and then used "Connection: keep-alive" (a HTTP/1.0 only feature/concept).

 

Is this over SSL/TLS perhaps?

If so, does this issue manifest without SSL/TLS?

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 4:04 PM, Daniel Potter <[hidden email]> wrote:

Hi Joakim,

 

Certainly, we’ll upgrade to 9.4.11 ASAP… but the issue is still present, unfortunately. Apologies for the outdated links, I’ve been chasing this bug for a while.

 

I was suspicious of the client at first, too (jersey 2.26), but the request doesn’t look wrong at all to me. Here’s an example of the snippet extracted by Wireshark (Auth token change is my edit, but this should still give a more clear example)—request first, followed by the response from jetty:

 

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1

Authorization: Bearer tokenWhichIsBase64

debug-request-id: ITEM-8fe57f6fcc81573a

User-Agent: Jersey/2.26 (HttpUrlConnection 1.8.0_172)

Host: localhost:18443

Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2

Connection: keep-alive

 

HTTP/1.1 400 Unknown Version

Content-Type: text/html;charset=iso-8859-1

Content-Length: 58

Connection: close

 

<h1>Bad Message 400</h1><pre>reason: Unknown Version</pre>

 

I’ll upgrade my local build to 9.4.11 and will keep trying to provide an isolated repro case.

 

Best,
Daniel

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 15:26
To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Also, what kind of client are you using to test with?

The description smells of a protocol bug introduced by a client. (the "unknown version" portion tells us the parse was parsing a line that didn't have a HTTP version)

 

If you have a setup that even occasionally produces this result we would like to see it.

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 2:51 PM, Daniel Potter <[hidden email]> wrote:

Hello jetty-dev,

 

I’ve been debugging a rather strange, timing-sensitive bug in a project that showed up after upgrading jetty from 9.2.13 -> 9.4.8 (still repro on 9.4.11), and have stumbled upon some suspicious behavior within jetty’s request handling/HTTP parsing layers. I haven’t been able to isolate a standalone repro case yet, but while I work on doing so, I was wondering if any of this sounded familiar or suspicious to anyone?

 

Summary of conditions to repro:

  • High volume of GET requests (200-400 per second)
  • A fairly small payload per request (around 500 bytes including all HTTP headers)

 

Observed repro:

  • Somewhat sporadically (usually between 5-10 minutes, although it can take up to 30), we see an HTTP 400 error returned to the client, due to “unknown version” (i.e., invalid HTTP version spec in the request). However, there is nothing unusual about the request that fails (compared to all the thousands that succeed). I’ve confirmed via Wireshark that the exact request in question is perfectly valid.

 

  • With similar probability, we sometimes see a duplicated response from an earlier request sent as the response to the subsequent request (so far, always on the same connection, but we haven’t ruled out that it might be happening across multiple connections). The end result is that all HTTP responses are shifted down by one, causing mayhem on the client side. According to Wireshark, the HTTP stream goes something like this:

    Request 1
    Response 1

    Request 2
    Response 2

    Request 3
    Response 2

    Request 4
    Response 3

    Request 5
    Response 4

 

I’ve narrowed the investigation down to what appears to be a race condition in filling on-heap buffers from an org.eclipse.jetty.io.ByteBufferPool with the HTTP request bytes, and parsing the request out of those same buffers. In particular, when the 400 “unknown version” error hits, org.eclipse.jetty.http.HttpParser.parseNext(buffer) is called with a buffer that appears to be in fill mode (position at end-of-content, limit at max capacity); yet by the time code has progressed to where the BAD_REQUEST_400 is thrown, the buffer has flipped back to flush mode (with position and limit both at the end-of-content). _methodString and _uri are both parsed to be gibberish (based on content that is read past the end of the buffer’s limit), and _version remains null.

 

Can anyone help confirm that the ByteBuffer behavior inside of HttpParser is unexpected? And, if by chance, this looks like anything else you all may have encountered?

 

Regards,
Daniel

 

Daniel Potter // Senior Software Engineer
PROS® // Powering Modern Commerce with Dynamic Pricing Science

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev
Reply | Threaded
Open this post in threaded view
|

Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

Joakim Erdfelt-8
What other changes have you made to your infrastructure on Jetty?
Are you using a custom ThreadPool perhaps? (meaning not the recommended QueuedThreadPool)
Or have you provided your own ByteBufferPool?
Or using a custom ServerConnector?
Or custom SelectorManager?
Or a custom cipher suite? (bouncy castle?)
Or are you running on an older OS? (such as a Linux kernel 3.x based distribution? or a minimal alpine based distro?)
Or are you running on an OS with known NIO select bugs? (eg: Microsoft Windows?)


Joakim Erdfelt / [hidden email]

On Tue, Jun 19, 2018 at 4:30 PM, Daniel Potter <[hidden email]> wrote:

Agreed—that’s definitely the client’s quirk. Still, it should be ignored and harmless when using HTTP/1.1 to spec, I believe?

 

I confirmed the bug both with, and without, TLS.

 

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 16:11


To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Strange request ...

 

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1

...

Connection: keep-alive

 

Your client specified "HTTP/1.1" and then used "Connection: keep-alive" (a HTTP/1.0 only feature/concept).

 

Is this over SSL/TLS perhaps?

If so, does this issue manifest without SSL/TLS?

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 4:04 PM, Daniel Potter <[hidden email]> wrote:

Hi Joakim,

 

Certainly, we’ll upgrade to 9.4.11 ASAP… but the issue is still present, unfortunately. Apologies for the outdated links, I’ve been chasing this bug for a while.

 

I was suspicious of the client at first, too (jersey 2.26), but the request doesn’t look wrong at all to me. Here’s an example of the snippet extracted by Wireshark (Auth token change is my edit, but this should still give a more clear example)—request first, followed by the response from jetty:

 

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1

Authorization: Bearer tokenWhichIsBase64

debug-request-id: ITEM-8fe57f6fcc81573a

User-Agent: Jersey/2.26 (HttpUrlConnection 1.8.0_172)

Host: localhost:18443

Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2

Connection: keep-alive

 

HTTP/1.1 400 Unknown Version

Content-Type: text/html;charset=iso-8859-1

Content-Length: 58

Connection: close

 

<h1>Bad Message 400</h1><pre>reason: Unknown Version</pre>

 

I’ll upgrade my local build to 9.4.11 and will keep trying to provide an isolated repro case.

 

Best,
Daniel

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 15:26
To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Also, what kind of client are you using to test with?

The description smells of a protocol bug introduced by a client. (the "unknown version" portion tells us the parse was parsing a line that didn't have a HTTP version)

 

If you have a setup that even occasionally produces this result we would like to see it.

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 2:51 PM, Daniel Potter <[hidden email]> wrote:

Hello jetty-dev,

 

I’ve been debugging a rather strange, timing-sensitive bug in a project that showed up after upgrading jetty from 9.2.13 -> 9.4.8 (still repro on 9.4.11), and have stumbled upon some suspicious behavior within jetty’s request handling/HTTP parsing layers. I haven’t been able to isolate a standalone repro case yet, but while I work on doing so, I was wondering if any of this sounded familiar or suspicious to anyone?

 

Summary of conditions to repro:

  • High volume of GET requests (200-400 per second)
  • A fairly small payload per request (around 500 bytes including all HTTP headers)

 

Observed repro:

  • Somewhat sporadically (usually between 5-10 minutes, although it can take up to 30), we see an HTTP 400 error returned to the client, due to “unknown version” (i.e., invalid HTTP version spec in the request). However, there is nothing unusual about the request that fails (compared to all the thousands that succeed). I’ve confirmed via Wireshark that the exact request in question is perfectly valid.

 

  • With similar probability, we sometimes see a duplicated response from an earlier request sent as the response to the subsequent request (so far, always on the same connection, but we haven’t ruled out that it might be happening across multiple connections). The end result is that all HTTP responses are shifted down by one, causing mayhem on the client side. According to Wireshark, the HTTP stream goes something like this:

    Request 1
    Response 1

    Request 2
    Response 2

    Request 3
    Response 2

    Request 4
    Response 3

    Request 5
    Response 4

 

I’ve narrowed the investigation down to what appears to be a race condition in filling on-heap buffers from an org.eclipse.jetty.io.ByteBufferPool with the HTTP request bytes, and parsing the request out of those same buffers. In particular, when the 400 “unknown version” error hits, org.eclipse.jetty.http.HttpParser.parseNext(buffer) is called with a buffer that appears to be in fill mode (position at end-of-content, limit at max capacity); yet by the time code has progressed to where the BAD_REQUEST_400 is thrown, the buffer has flipped back to flush mode (with position and limit both at the end-of-content). _methodString and _uri are both parsed to be gibberish (based on content that is read past the end of the buffer’s limit), and _version remains null.

 

Can anyone help confirm that the ByteBuffer behavior inside of HttpParser is unexpected? And, if by chance, this looks like anything else you all may have encountered?

 

Regards,
Daniel

 

Daniel Potter // Senior Software Engineer
PROS® // Powering Modern Commerce with Dynamic Pricing Science

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev
Reply | Threaded
Open this post in threaded view
|

Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

Daniel Potter

OS: we’re running in a dockerized Debian 9 distro, Linux 4.9.87-linuxkit-aufs.

 

We’re using jetty as “configured” by dropwizard. It uses com.codahale.metrics.jetty9.InstrumentedQueuedThreadPool, which looks like a thin subclass of org.eclipse.jetty.util.QueuedThreadPool to export metrics.

 

Pretty much everything else is jetty’s defaults:

ByteBufferPool is org.eclipse.jetty.io.ArrayByteBufferPool

ServerConnector is org.eclipse.jetty.server.ServerConnector

SelectorManager is org.eclipse.jetty.io.SelectorManager

Cipher suite is not custom, using JVM default-provided one.

 

Regards,

Daniel

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 17:00
To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

What other changes have you made to your infrastructure on Jetty?

Are you using a custom ThreadPool perhaps? (meaning not the recommended QueuedThreadPool)

Or have you provided your own ByteBufferPool?

Or using a custom ServerConnector?

Or custom SelectorManager?

Or a custom cipher suite? (bouncy castle?)

Or are you running on an older OS? (such as a Linux kernel 3.x based distribution? or a minimal alpine based distro?)

Or are you running on an OS with known NIO select bugs? (eg: Microsoft Windows?)

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 4:30 PM, Daniel Potter <[hidden email]> wrote:

Agreed—that’s definitely the client’s quirk. Still, it should be ignored and harmless when using HTTP/1.1 to spec, I believe?

 

I confirmed the bug both with, and without, TLS.

 

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 16:11


To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Strange request ...

 

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1

...

Connection: keep-alive

 

Your client specified "HTTP/1.1" and then used "Connection: keep-alive" (a HTTP/1.0 only feature/concept).

 

Is this over SSL/TLS perhaps?

If so, does this issue manifest without SSL/TLS?

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 4:04 PM, Daniel Potter <[hidden email]> wrote:

Hi Joakim,

 

Certainly, we’ll upgrade to 9.4.11 ASAP… but the issue is still present, unfortunately. Apologies for the outdated links, I’ve been chasing this bug for a while.

 

I was suspicious of the client at first, too (jersey 2.26), but the request doesn’t look wrong at all to me. Here’s an example of the snippet extracted by Wireshark (Auth token change is my edit, but this should still give a more clear example)—request first, followed by the response from jetty:

 

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1

Authorization: Bearer tokenWhichIsBase64

debug-request-id: ITEM-8fe57f6fcc81573a

User-Agent: Jersey/2.26 (HttpUrlConnection 1.8.0_172)

Host: localhost:18443

Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2

Connection: keep-alive

 

HTTP/1.1 400 Unknown Version

Content-Type: text/html;charset=iso-8859-1

Content-Length: 58

Connection: close

 

<h1>Bad Message 400</h1><pre>reason: Unknown Version</pre>

 

I’ll upgrade my local build to 9.4.11 and will keep trying to provide an isolated repro case.

 

Best,
Daniel

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 15:26
To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Also, what kind of client are you using to test with?

The description smells of a protocol bug introduced by a client. (the "unknown version" portion tells us the parse was parsing a line that didn't have a HTTP version)

 

If you have a setup that even occasionally produces this result we would like to see it.

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 2:51 PM, Daniel Potter <[hidden email]> wrote:

Hello jetty-dev,

 

I’ve been debugging a rather strange, timing-sensitive bug in a project that showed up after upgrading jetty from 9.2.13 -> 9.4.8 (still repro on 9.4.11), and have stumbled upon some suspicious behavior within jetty’s request handling/HTTP parsing layers. I haven’t been able to isolate a standalone repro case yet, but while I work on doing so, I was wondering if any of this sounded familiar or suspicious to anyone?

 

Summary of conditions to repro:

  • High volume of GET requests (200-400 per second)
  • A fairly small payload per request (around 500 bytes including all HTTP headers)

 

Observed repro:

  • Somewhat sporadically (usually between 5-10 minutes, although it can take up to 30), we see an HTTP 400 error returned to the client, due to “unknown version” (i.e., invalid HTTP version spec in the request). However, there is nothing unusual about the request that fails (compared to all the thousands that succeed). I’ve confirmed via Wireshark that the exact request in question is perfectly valid.

 

  • With similar probability, we sometimes see a duplicated response from an earlier request sent as the response to the subsequent request (so far, always on the same connection, but we haven’t ruled out that it might be happening across multiple connections). The end result is that all HTTP responses are shifted down by one, causing mayhem on the client side. According to Wireshark, the HTTP stream goes something like this:

    Request 1
    Response 1

    Request 2
    Response 2

    Request 3
    Response 2

    Request 4
    Response 3

    Request 5
    Response 4

 

I’ve narrowed the investigation down to what appears to be a race condition in filling on-heap buffers from an org.eclipse.jetty.io.ByteBufferPool with the HTTP request bytes, and parsing the request out of those same buffers. In particular, when the 400 “unknown version” error hits, org.eclipse.jetty.http.HttpParser.parseNext(buffer) is called with a buffer that appears to be in fill mode (position at end-of-content, limit at max capacity); yet by the time code has progressed to where the BAD_REQUEST_400 is thrown, the buffer has flipped back to flush mode (with position and limit both at the end-of-content). _methodString and _uri are both parsed to be gibberish (based on content that is read past the end of the buffer’s limit), and _version remains null.

 

Can anyone help confirm that the ByteBuffer behavior inside of HttpParser is unexpected? And, if by chance, this looks like anything else you all may have encountered?

 

Regards,
Daniel

 

Daniel Potter // Senior Software Engineer
PROS® // Powering Modern Commerce with Dynamic Pricing Science

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev
Reply | Threaded
Open this post in threaded view
|

Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

Greg Wilkins
Daniel,

what you describe does indeed sound incorrect.  It sounds very much like a buffer has been returned to the buffer pool when it should not have been.  We have occasionally had bugs like that, but I've not seen a report for one in 9.4.8.

We do have a class LeakTrackingByteBufferPool, which we can use to check that buffers are not being doubly allocated.  Tell me a bit more about how you setup your server and I'll be able to inform you how to use that pool.... but still upgrade to 9.4.11

regards


On 20 June 2018 at 01:40, Daniel Potter <[hidden email]> wrote:

OS: we’re running in a dockerized Debian 9 distro, Linux 4.9.87-linuxkit-aufs.

 

We’re using jetty as “configured” by dropwizard. It uses com.codahale.metrics.jetty9.InstrumentedQueuedThreadPool, which looks like a thin subclass of org.eclipse.jetty.util.QueuedThreadPool to export metrics.

 

Pretty much everything else is jetty’s defaults:

ByteBufferPool is org.eclipse.jetty.io.ArrayByteBufferPool

ServerConnector is org.eclipse.jetty.server.ServerConnector

SelectorManager is org.eclipse.jetty.io.SelectorManager

Cipher suite is not custom, using JVM default-provided one.

 

Regards,

Daniel

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 17:00


To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

What other changes have you made to your infrastructure on Jetty?

Are you using a custom ThreadPool perhaps? (meaning not the recommended QueuedThreadPool)

Or have you provided your own ByteBufferPool?

Or using a custom ServerConnector?

Or custom SelectorManager?

Or a custom cipher suite? (bouncy castle?)

Or are you running on an older OS? (such as a Linux kernel 3.x based distribution? or a minimal alpine based distro?)

Or are you running on an OS with known NIO select bugs? (eg: Microsoft Windows?)

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 4:30 PM, Daniel Potter <[hidden email]> wrote:

Agreed—that’s definitely the client’s quirk. Still, it should be ignored and harmless when using HTTP/1.1 to spec, I believe?

 

I confirmed the bug both with, and without, TLS.

 

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 16:11


To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Strange request ...

 

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1

...

Connection: keep-alive

 

Your client specified "HTTP/1.1" and then used "Connection: keep-alive" (a HTTP/1.0 only feature/concept).

 

Is this over SSL/TLS perhaps?

If so, does this issue manifest without SSL/TLS?

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 4:04 PM, Daniel Potter <[hidden email]> wrote:

Hi Joakim,

 

Certainly, we’ll upgrade to 9.4.11 ASAP… but the issue is still present, unfortunately. Apologies for the outdated links, I’ve been chasing this bug for a while.

 

I was suspicious of the client at first, too (jersey 2.26), but the request doesn’t look wrong at all to me. Here’s an example of the snippet extracted by Wireshark (Auth token change is my edit, but this should still give a more clear example)—request first, followed by the response from jetty:

 

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1

Authorization: Bearer tokenWhichIsBase64

debug-request-id: ITEM-8fe57f6fcc81573a

User-Agent: Jersey/2.26 (HttpUrlConnection 1.8.0_172)

Host: localhost:18443

Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2

Connection: keep-alive

 

HTTP/1.1 400 Unknown Version

Content-Type: text/html;charset=iso-8859-1

Content-Length: 58

Connection: close

 

<h1>Bad Message 400</h1><pre>reason: Unknown Version</pre>

 

I’ll upgrade my local build to 9.4.11 and will keep trying to provide an isolated repro case.

 

Best,
Daniel

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 15:26
To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Also, what kind of client are you using to test with?

The description smells of a protocol bug introduced by a client. (the "unknown version" portion tells us the parse was parsing a line that didn't have a HTTP version)

 

If you have a setup that even occasionally produces this result we would like to see it.

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 2:51 PM, Daniel Potter <[hidden email]> wrote:

Hello jetty-dev,

 

I’ve been debugging a rather strange, timing-sensitive bug in a project that showed up after upgrading jetty from 9.2.13 -> 9.4.8 (still repro on 9.4.11), and have stumbled upon some suspicious behavior within jetty’s request handling/HTTP parsing layers. I haven’t been able to isolate a standalone repro case yet, but while I work on doing so, I was wondering if any of this sounded familiar or suspicious to anyone?

 

Summary of conditions to repro:

  • High volume of GET requests (200-400 per second)
  • A fairly small payload per request (around 500 bytes including all HTTP headers)

 

Observed repro:

  • Somewhat sporadically (usually between 5-10 minutes, although it can take up to 30), we see an HTTP 400 error returned to the client, due to “unknown version” (i.e., invalid HTTP version spec in the request). However, there is nothing unusual about the request that fails (compared to all the thousands that succeed). I’ve confirmed via Wireshark that the exact request in question is perfectly valid.

 

  • With similar probability, we sometimes see a duplicated response from an earlier request sent as the response to the subsequent request (so far, always on the same connection, but we haven’t ruled out that it might be happening across multiple connections). The end result is that all HTTP responses are shifted down by one, causing mayhem on the client side. According to Wireshark, the HTTP stream goes something like this:

    Request 1
    Response 1

    Request 2
    Response 2

    Request 3
    Response 2

    Request 4
    Response 3

    Request 5
    Response 4

 

I’ve narrowed the investigation down to what appears to be a race condition in filling on-heap buffers from an org.eclipse.jetty.io.ByteBufferPool with the HTTP request bytes, and parsing the request out of those same buffers. In particular, when the 400 “unknown version” error hits, org.eclipse.jetty.http.HttpParser.parseNext(buffer) is called with a buffer that appears to be in fill mode (position at end-of-content, limit at max capacity); yet by the time code has progressed to where the BAD_REQUEST_400 is thrown, the buffer has flipped back to flush mode (with position and limit both at the end-of-content). _methodString and _uri are both parsed to be gibberish (based on content that is read past the end of the buffer’s limit), and _version remains null.

 

Can anyone help confirm that the ByteBuffer behavior inside of HttpParser is unexpected? And, if by chance, this looks like anything else you all may have encountered?

 

Regards,
Daniel

 

Daniel Potter // Senior Software Engineer
PROS® // Powering Modern Commerce with Dynamic Pricing Science

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev



--

_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev
Reply | Threaded
Open this post in threaded view
|

Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

Daniel Potter

Hi Greg,

 

Thanks for acknowledging that this sounds incorrect—I’m glad if I indeed have not been chasing a false lead.

 

I’ve rebuilt my repro locally on top of 9.4.11, and re-confirmed that the same exact symptoms are still present.

 

Thank you for pointing out the availability of LeakTrackingByteBufferPool, I didn’t realize that such already existed! It looks like for detecting double releases/acquires, all I had to do was to wrap the ArrayByteBufferPool instance in a LeakTrackingByteBufferPool, right? I tried doing so by modifying the original code here, and confirmed with a debugger that the ByteBuffer instances were now being tracked to detect incorrect acquires/releases. Sadly, the bug triggered *without* the leak detection alerting on anything. So there’s not a simple double-release going on, from what I can tell.

 

I haven’t had any more luck identifying what thread is causing this background mutation, or why. I’ll keep trying to track this down, and let you all know if I succeed, or if I manage to create a standalone repro demonstrating this same issue.

 

Regards,

Daniel

 

 

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Greg Wilkins
Sent: Wednesday, June 20, 2018 15:59
To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Daniel,

 

what you describe does indeed sound incorrect.  It sounds very much like a buffer has been returned to the buffer pool when it should not have been.  We have occasionally had bugs like that, but I've not seen a report for one in 9.4.8.

 

We do have a class LeakTrackingByteBufferPool, which we can use to check that buffers are not being doubly allocated.  Tell me a bit more about how you setup your server and I'll be able to inform you how to use that pool.... but still upgrade to 9.4.11

 

regards

 

 

On 20 June 2018 at 01:40, Daniel Potter <[hidden email]> wrote:

OS: we’re running in a dockerized Debian 9 distro, Linux 4.9.87-linuxkit-aufs.

 

We’re using jetty as “configured” by dropwizard. It uses com.codahale.metrics.jetty9.InstrumentedQueuedThreadPool, which looks like a thin subclass of org.eclipse.jetty.util.QueuedThreadPool to export metrics.

 

Pretty much everything else is jetty’s defaults:

ByteBufferPool is org.eclipse.jetty.io.ArrayByteBufferPool

ServerConnector is org.eclipse.jetty.server.ServerConnector

SelectorManager is org.eclipse.jetty.io.SelectorManager

Cipher suite is not custom, using JVM default-provided one.

 

Regards,

Daniel

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 17:00


To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

What other changes have you made to your infrastructure on Jetty?

Are you using a custom ThreadPool perhaps? (meaning not the recommended QueuedThreadPool)

Or have you provided your own ByteBufferPool?

Or using a custom ServerConnector?

Or custom SelectorManager?

Or a custom cipher suite? (bouncy castle?)

Or are you running on an older OS? (such as a Linux kernel 3.x based distribution? or a minimal alpine based distro?)

Or are you running on an OS with known NIO select bugs? (eg: Microsoft Windows?)

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 4:30 PM, Daniel Potter <[hidden email]> wrote:

Agreed—that’s definitely the client’s quirk. Still, it should be ignored and harmless when using HTTP/1.1 to spec, I believe?

 

I confirmed the bug both with, and without, TLS.

 

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 16:11


To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Strange request ...

 

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1

...

Connection: keep-alive

 

Your client specified "HTTP/1.1" and then used "Connection: keep-alive" (a HTTP/1.0 only feature/concept).

 

Is this over SSL/TLS perhaps?

If so, does this issue manifest without SSL/TLS?

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 4:04 PM, Daniel Potter <[hidden email]> wrote:

Hi Joakim,

 

Certainly, we’ll upgrade to 9.4.11 ASAP… but the issue is still present, unfortunately. Apologies for the outdated links, I’ve been chasing this bug for a while.

 

I was suspicious of the client at first, too (jersey 2.26), but the request doesn’t look wrong at all to me. Here’s an example of the snippet extracted by Wireshark (Auth token change is my edit, but this should still give a more clear example)—request first, followed by the response from jetty:

 

GET /api/groups/id/c689acc7-ee13-3482-aa7b-c6a13419431c/items/name/G6-A7-Sample HTTP/1.1

Authorization: Bearer tokenWhichIsBase64

debug-request-id: ITEM-8fe57f6fcc81573a

User-Agent: Jersey/2.26 (HttpUrlConnection 1.8.0_172)

Host: localhost:18443

Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2

Connection: keep-alive

 

HTTP/1.1 400 Unknown Version

Content-Type: text/html;charset=iso-8859-1

Content-Length: 58

Connection: close

 

<h1>Bad Message 400</h1><pre>reason: Unknown Version</pre>

 

I’ll upgrade my local build to 9.4.11 and will keep trying to provide an isolated repro case.

 

Best,
Daniel

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Joakim Erdfelt
Sent: Tuesday, June 19, 2018 15:26
To: Jetty @ Eclipse developer discussion list <[hidden email]>
Subject: Re: [jetty-dev] Possible race condition with ByteBuffers in HttpParser

 

Also, what kind of client are you using to test with?

The description smells of a protocol bug introduced by a client. (the "unknown version" portion tells us the parse was parsing a line that didn't have a HTTP version)

 

If you have a setup that even occasionally produces this result we would like to see it.

 


Joakim Erdfelt / [hidden email]

 

On Tue, Jun 19, 2018 at 2:51 PM, Daniel Potter <[hidden email]> wrote:

Hello jetty-dev,

 

I’ve been debugging a rather strange, timing-sensitive bug in a project that showed up after upgrading jetty from 9.2.13 -> 9.4.8 (still repro on 9.4.11), and have stumbled upon some suspicious behavior within jetty’s request handling/HTTP parsing layers. I haven’t been able to isolate a standalone repro case yet, but while I work on doing so, I was wondering if any of this sounded familiar or suspicious to anyone?

 

Summary of conditions to repro:

  • High volume of GET requests (200-400 per second)
  • A fairly small payload per request (around 500 bytes including all HTTP headers)

 

Observed repro:

  • Somewhat sporadically (usually between 5-10 minutes, although it can take up to 30), we see an HTTP 400 error returned to the client, due to “unknown version” (i.e., invalid HTTP version spec in the request). However, there is nothing unusual about the request that fails (compared to all the thousands that succeed). I’ve confirmed via Wireshark that the exact request in question is perfectly valid.

 

  • With similar probability, we sometimes see a duplicated response from an earlier request sent as the response to the subsequent request (so far, always on the same connection, but we haven’t ruled out that it might be happening across multiple connections). The end result is that all HTTP responses are shifted down by one, causing mayhem on the client side. According to Wireshark, the HTTP stream goes something like this:

    Request 1
    Response 1

    Request 2
    Response 2

    Request 3
    Response 2

    Request 4
    Response 3

    Request 5
    Response 4

 

I’ve narrowed the investigation down to what appears to be a race condition in filling on-heap buffers from an org.eclipse.jetty.io.ByteBufferPool with the HTTP request bytes, and parsing the request out of those same buffers. In particular, when the 400 “unknown version” error hits, org.eclipse.jetty.http.HttpParser.parseNext(buffer) is called with a buffer that appears to be in fill mode (position at end-of-content, limit at max capacity); yet by the time code has progressed to where the BAD_REQUEST_400 is thrown, the buffer has flipped back to flush mode (with position and limit both at the end-of-content). _methodString and _uri are both parsed to be gibberish (based on content that is read past the end of the buffer’s limit), and _version remains null.

 

Can anyone help confirm that the ByteBuffer behavior inside of HttpParser is unexpected? And, if by chance, this looks like anything else you all may have encountered?

 

Regards,
Daniel

 

Daniel Potter // Senior Software Engineer
PROS® // Powering Modern Commerce with Dynamic Pricing Science

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev

 


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev



 

--


_______________________________________________
jetty-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-dev