New issue
Advanced search Search tips

Issue 752773 link

Starred by 1 user
Status: Fixed
Owner:
mmenke@chromium.org
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-Servicification



Sign in to add a comment

URLLoaderImpl doesn't correctly handle errors received after reading headers.

Project Member Reported by mmenke@chromium.org, Aug 5 2017 
If URLLoaderImpl synchronously encounters an error when reading the response body, it calls "NotifyCompleted(net::OK);" instead of passing the error along.  If it instead encounters the error asynchronously, in OnReadCompleted, it just closes the URLLoader pipe, instead of passing the error code along.

I also think this indicates the class isn't adequately tested (And skimming over the tests, it looks like it doesn't have any testing for errors at all).
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 9 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1dd3f2f39bf1b6b605f9227c886b47ccbd850179

commit 1dd3f2f39bf1b6b605f9227c886b47ccbd850179
Author: Matt Menke <mmenke@chromium.org>
Date: Wed Aug 09 17:10:06 2017

URLLoaderImpl:  Fix handling of read errors.

On an sync read error, transmit the error instead of a successful
completion.  On an async read error, transmit the error instead of just
closing the pipe.

Also fix how ResourceRequestCompletionStatus::decoded_body_length, is
calculated on errors.

Bug:  752773 
Change-Id: Id42db09e2b50717e1a625aaa6577a0f9ebcd1ce1
Reviewed-on: https://chromium-review.googlesource.com/603971
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Randy Smith <rdsmith@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493031}
[modify] https://crrev.com/1dd3f2f39bf1b6b605f9227c886b47ccbd850179/content/network/url_loader_impl.cc
[modify] https://crrev.com/1dd3f2f39bf1b6b605f9227c886b47ccbd850179/content/network/url_loader_impl.h
[modify] https://crrev.com/1dd3f2f39bf1b6b605f9227c886b47ccbd850179/content/network/url_loader_unittest.cc
[modify] https://crrev.com/1dd3f2f39bf1b6b605f9227c886b47ccbd850179/content/test/data/content-sniffer-test0.html.mock-http-headers
[modify] https://crrev.com/1dd3f2f39bf1b6b605f9227c886b47ccbd850179/content/test/data/content-sniffer-test1.html.mock-http-headers
[modify] https://crrev.com/1dd3f2f39bf1b6b605f9227c886b47ccbd850179/content/test/data/content-sniffer-test2.html.mock-http-headers
[modify] https://crrev.com/1dd3f2f39bf1b6b605f9227c886b47ccbd850179/content/test/data/content-sniffer-test3.html.mock-http-headers
[modify] https://crrev.com/1dd3f2f39bf1b6b605f9227c886b47ccbd850179/net/BUILD.gn
[modify] https://crrev.com/1dd3f2f39bf1b6b605f9227c886b47ccbd850179/net/test/embedded_test_server/default_handlers.cc

Comment 2 by mmenke@chromium.org, Aug 9 2017

Status: Fixed (was: Assigned)

Comment 3 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
►

Sign in to add a comment