New issue
Advanced search Search tips

Issue 811717 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Event-based CBOR parser for streaming consumers

Project Member Reported by kouhei@chromium.org, Feb 13 2018

Issue description

Currently CBOR consumers which handles large CBOR bytestream relies on a hacky interface which exposes details of CBOR header token.

We should remove the hack and implement an event-based SAX-like CBOR parser interface.
 

Comment 1 by engedy@chromium.org, Feb 13 2018

Components: Blink>WebAuthentication
Adding the only other user of the CBORReader as the component.
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 20 2018

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

commit 8abc3038960de43eb28bfe10cd6c058709dc9aad
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Tue Feb 20 05:36:19 2018

CBORReader: Add CBORReader::DecodeDataItemHeader()

This CL introduces *experimental* CBORReader::ReadDataItemHeader()
method which allows streaming consuming of a large CBOR-encoded array.

Since this API exposes CBORReader::DataItemHeader, which should conceptually be
encapsulated inside CBORReader, the method should not be used widely, and
should be replaced by event-based CBORReader API.

The method is really only for short-term use from SignedExchangeParser, which
definitely requires streaming parsing, but is currently unclear if it will
continue to use CBOR encoding in long term.

Bug: 803774,  811717 
Change-Id: If37c70b034afcdecbe6e7793c95b4e1f60eee9d0
Reviewed-on: https://chromium-review.googlesource.com/920723
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537721}
[modify] https://crrev.com/8abc3038960de43eb28bfe10cd6c058709dc9aad/components/cbor/cbor_reader.cc
[modify] https://crrev.com/8abc3038960de43eb28bfe10cd6c058709dc9aad/components/cbor/cbor_reader.h
[modify] https://crrev.com/8abc3038960de43eb28bfe10cd6c058709dc9aad/components/cbor/cbor_reader_unittest.cc

Comment 3 by engedy@chromium.org, Mar 31 2018

Status: WontFix (was: Untriaged)
Closing this as there are no consumers who'd need this in the foreseeable future. Feel free to re-open if the need arises.
I'll revert the change, since the need for streaming CBOR was removed from the signedexchange spec too.
Cc: -kouhei@chromium.org
Labels: Hotlist-WebAuthnFixit
Owner: kouhei@chromium.org
Status: Assigned (was: WontFix)
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 20 2018

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

commit 4fb1aebca0d4909e3fc560f0d6295823d98ba50f
Author: Kouhei Ueno <kouhei@chromium.org>
Date: Fri Apr 20 16:12:00 2018

Revert CBORReader::ReadDataItemHeader

This CL removes an unused method.
We originally planned to use the method to parse SignedExchanges, but they no longer require this.

Bug: 803774,  811717 
Change-Id: I5bd3f89891f046e4b6b1f128a63584cd78918266
Reviewed-on: https://chromium-review.googlesource.com/1017420
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552350}
[modify] https://crrev.com/4fb1aebca0d4909e3fc560f0d6295823d98ba50f/components/cbor/cbor_reader.cc
[modify] https://crrev.com/4fb1aebca0d4909e3fc560f0d6295823d98ba50f/components/cbor/cbor_reader.h
[modify] https://crrev.com/4fb1aebca0d4909e3fc560f0d6295823d98ba50f/components/cbor/cbor_reader_unittest.cc

Comment 7 by kouhei@chromium.org, Apr 20 2018

Status: Fixed (was: Assigned)

Sign in to add a comment