New issue
Advanced search Search tips

Issue 813561 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

Extension web request api signals DCHECK when working with requests with binary data in form data

Reported by ganenk...@yandex-team.ru, Feb 19 2018

Issue description

Chrome Version       : any
OS Version: any

What steps will reproduce the problem?
Use extension web request API when listening requests with binary data in form data.

What is the expected result?
Binary data passed with request should be properly monitored.

What happens instead of that?
Binary data were stored as utf-8 string in base::Value DCHECK'ing with debug builds. In release builds not obvious work with Value::Type::STRING (that is not utf-8) could lead to difficult to reproduce bugs.





 
Components: Platform>Extensions
Having the exact stack trace for the dcheck will probably help in resolving this. Please post it.
Labels: Needs-Milestone
Stack:
[15651:15682:0116/154553.806045:FATAL:values.cc(148)] Check failed: IsStringUTF8(string_value_). 
#0 0x7fece4e0da6c base::debug::StackTrace::StackTrace()
#1 0x7fece4e37a8c logging::LogMessage::~LogMessage()
#2 0x7fece4eebe00 base::Value::Value()
#3 0x563cd43d8b1e _ZNSt3__16vectorIN4base5ValueENS_9allocatorIS2_EEE24__emplace_back_slow_pathIJRKNS_12basic_stringIcNS_11char_traitsIcEENS3_IcEEEEEEEvDpOT_
#4 0x563cd43d894c extensions::ParsedDataPresenter::FeedNext()
#5 0x563cd43ecc30 extensions::WebRequestInfo::WebRequestInfo()
#6 0x563cd47f0407 (anonymous namespace)::ChromeExtensionsNetworkDelegateImpl::OnBeforeURLRequest()
#7 0x563cd47ef07f ChromeNetworkDelegate::OnBeforeURLRequest()
#8 0x7fece4451121 net::NetworkDelegate::NotifyBeforeURLRequest()
#9 0x7fece4451121 net::NetworkDelegate::NotifyBeforeURLRequest()
#10 0x7fece4451121 net::NetworkDelegate::NotifyBeforeURLRequest()
#11 0x7fece475bfb0 net::URLRequest::Start()
#12 0x7fece4754d55 net::URLFetcherCore::StartURLRequest()
#13 0x7fece47553e9 net::URLFetcherCore::StartURLRequestWhenAppropriate()
#14 0x7fece47508e4 net::URLFetcherCore::StartOnIOThread()
#15 0x7fece4479b5b _ZN4base8internal7InvokerINS0_9BindStateIMN3net12_GLOBAL__N_111RequestCoreEFvvEJ13scoped_refptrIS5_EEEEFvvEE3RunEPNS0_13BindStateBaseE
#16 0x7fece4e0e38a base::debug::TaskAnnotator::RunTask()
#17 0x7fece4e41386 base::internal::IncomingTaskQueue::RunTask()
#18 0x7fece4f0bb57 base::MessageLoop::RunTask()
#19 0x7fece4f0bf74 base::MessageLoop::DeferOrRunPendingTask()
#20 0x7fece4f0c238 base::MessageLoop::DoWork()
#21 0x7fece4e452f9 base::MessagePumpLibevent::Run()
#22 0x7fece4f0b39c base::MessageLoop::Run()
#23 0x7fece4f0d8d6 base::RunLoop::Run()
#24 0x7fece4eb981a base::Thread::Run()
#25 0x7fece2cf5298 content::BrowserThreadImpl::IOThreadRun()
#26 0x7fece2cf54b1 content::BrowserThreadImpl::Run()
#27 0x7fece4eb9e13 base::Thread::ThreadMain()
#28 0x7fece4eb015f base::(anonymous namespace)::ThreadFunc()
#29 0x7fece4f67184 start_thread
#30 0x7fecd7b1c37d clone
Labels: Triaged-ET TE-NeedsTriageHelp
As this issue is related to DCHECK, adding TE-NeedsTriageHelp for further help in triaging this issue.

Thanks..
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 28 2018

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

commit 787994ad617d5b06ad340de236ebeb9d18795f93
Author: Konstantin Ganenko <ganenkokb@yandex-team.ru>
Date: Wed Feb 28 18:54:37 2018

Fix parsing form data binary data.

FormDataParserMultipart contract says that "*OCTET" part of body is read
 and passed as string of bytes. In case header has filename, octet part
 is ignored - store filename instead.
Actual code doesn't implement it. It led to situation when octet data
 were read and were put as string to base::Value. That led to DCHECK as far
 as string value must be utf-8 string.
The same problem occurs with FormDataParserUrlEncoded, where encoded part
 can be as utf-8 string or binary data.

Bug:  813561 
Change-Id: I453e5767e44334535253d1bd3fa4e857c2d3a3ff
Reviewed-on: https://chromium-review.googlesource.com/910848
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539907}
[modify] https://crrev.com/787994ad617d5b06ad340de236ebeb9d18795f93/chrome/browser/extensions/api/web_request/web_request_api_unittest.cc
[modify] https://crrev.com/787994ad617d5b06ad340de236ebeb9d18795f93/extensions/browser/api/web_request/form_data_parser.cc
[modify] https://crrev.com/787994ad617d5b06ad340de236ebeb9d18795f93/extensions/browser/api/web_request/form_data_parser.h
[modify] https://crrev.com/787994ad617d5b06ad340de236ebeb9d18795f93/extensions/browser/api/web_request/form_data_parser_unittest.cc
[modify] https://crrev.com/787994ad617d5b06ad340de236ebeb9d18795f93/extensions/browser/api/web_request/upload_data_presenter.cc
[modify] https://crrev.com/787994ad617d5b06ad340de236ebeb9d18795f93/extensions/common/api/web_request.json
[modify] https://crrev.com/787994ad617d5b06ad340de236ebeb9d18795f93/extensions/renderer/resources/test_custom_bindings.js

Cc: ganenk...@yandex-team.ru
Status: Started (was: Unconfirmed)
Is there any work left here or can this issue be closed?
Issue can be closed
Labels: -TE-NeedsTriageHelp -Needs-Milestone -Triaged-ET M-66
Status: Fixed (was: Started)

Sign in to add a comment