Issue metadata
Sign in to add a comment
|
4xx/5xx requests keep pending when their resonse contain control characters
Reported by
l446240525@gmail.com,
Jul 1 2017
|
||||||||||||||||||||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3143.0 Safari/537.36
Example URL:
Steps to reproduce the problem:
repro:
Create a simple http server with Koa (a Node.js framework):
const Koa = require('koa');
const app = new Koa();
app.use(ctx => {
try {
JSON.parse(ctx.query.foo);
} catch (e) {
ctx.throw(403, e);
}
});
app.listen(3000);
then open http://127.0.0.1:3000/?foo=%00 in Chrome
What is the expected behavior?
What went wrong?
A more minimal repro (shell script):
echo $'HTTP/1.1 403 Forbidden\r\ncontent-type: text/plain;\r\n\r\n\x01' | nc -l 3000
then open http://127.0.0.1:3000 in Chrome
Did this work before? Yes
Chrome version: 61.0.3143.0 Channel: n/a
OS Version: OS X 10.12.5
Flash Version:
,
Jul 5 2017
Is the server closing the connection? If not, this may be expected behavior, since we don't know the response has ended yet, since there's no content length.
,
Jul 5 2017
The server's closing the connection. (echo eventually terminates.) But I may just be getting confused because URL_REQUEST_JOB_FILTERED_BYTES_READ just doesn't log result=0. So this might be a problem in content/browser/loader.
,
Jul 5 2017
I've got a regression range now and am just going to run a bisect.
,
Jul 5 2017
Bisect gave: https://chromium.googlesource.com/chromium/src/+log/8422bf10bc4af33a0db2fde5117792ecde362863..384cc18b105bd3ffefe5339d75edd8e3dec7d927 which is probably this: https://chromium.googlesource.com/chromium/src/+/87f5c77aaa3541bee2f8d3b6b64d3879b8e81957 Matt, mind taking a look?
,
Jul 5 2017
Happy day! I won the bug! Sure, will do. Probably the MIME Sniffer.
,
Jul 6 2017
I bet this is this downloads path - despite the mime type, we still sniff on text/plain. Then we see it's a binary file, so we sniff as binary/octet stream. So it's a download! But downloading errors to files is weird, so we abort the navigation. My CL probably broke that logic in some way. So assuming I'm right (I have reproed, and confirmed it doesn't appear with a 200 response code, but haven't dug into the code where things are going wrong), this is worth fixing, but I don't think it needs to be merged to M60.
,
Jul 6 2017
And I've confirmed that's the issue. Should be able to land a fix before M61 branches. Thanks for the detailed report, l446240525!
,
Jul 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/72fd4f4bc6752250814df9ca73ab9786abc667dd commit 72fd4f4bc6752250814df9ca73ab9786abc667dd Author: mmenke <mmenke@chromium.org> Date: Thu Jul 06 20:03:38 2017 Fix short error responses sniffed as downloads hanging. When MimeSniffingResourceHandler sniffs an error responses request as a download, it refuses to download the file and cancels the request with an error instead. It used to both pass cancellation up through the ResourceHandler and cancel the URLRequest itself. However, as of https://codereview.chromium.org/2526983002, it just cancelled the request, rather than passing the cancellation message up the ResourceHandler stack. This mostly works...Except in the case we had to read to the end of the response before we could sniff the mime type. Completed requests do nothing when canceled, so the request would hang. Without the notification up through the ResourceHandler stack, the ResourceLoader never learns of cancellation. This CL replaces cancelling the URLRequest directly with cancelling through the ResourceController (Which is what it should have been doing, anyways). BUG= 738680 Review-Url: https://codereview.chromium.org/2974493002 Cr-Commit-Position: refs/heads/master@{#484720} [modify] https://crrev.com/72fd4f4bc6752250814df9ca73ab9786abc667dd/chrome/browser/net/errorpage_browsertest.cc [modify] https://crrev.com/72fd4f4bc6752250814df9ca73ab9786abc667dd/content/browser/loader/mime_sniffing_resource_handler.cc
,
Jul 6 2017
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by davidben@chromium.org
, Jul 5 2017Status: Untriaged (was: Unconfirmed)