New issue
Advanced search Search tips

Issue 778263 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Headless page IDs in Canary can't read encoded IDs

Reported by br...@dockyard.com, Oct 25 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.62 Safari/537.36

Steps to reproduce the problem:
1. When using the HTTP API for managing headless some libraries will normalize request URLs
2. Canary uses page IDs wrapped in parens "(ABCD1234)"
3. This can result in a request like "/json/close%28ABCD1234%29" which should be a valid URL but Chrome doesn't attempt to decode the ID

What is the expected behavior?
I would expect the Chrome API to decode the ID as any HTTP endpoint should

What went wrong?
Because the ID is not being decoded the page of the given ID is never found and the result will always be 404

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version:  64.0.3249.0  Channel: stable
OS Version: OS X 10.12.6
Flash Version:
 

Comment 1 by lgrey@chromium.org, Oct 25 2017

Labels: Proj-Headless
Components: Internals>Headless
Components: Platform>DevTools
Status: Available (was: Unconfirmed)
Did we change the representation of page ids recently?

Comment 4 by br...@dockyard.com, Oct 27 2017

Between beta and canary the page IDs are now wrapped in parens

Comment 5 by br...@dockyard.com, Oct 27 2017

I don't know if this was intentional or not
Owner: pfeldman@chromium.org
Status: Assigned (was: Available)
We did change the format from guid to unguessable token. We can decode it in the http handler, but it would only manifest itself if user encodes the URL obtained at /json/list. As long as the user does not do that, we should be fine. It is already in Beta I believe.

Comment 7 by br...@dockyard.com, Oct 27 2017

@pfeldman@chromium.org

the issue we're running into is that we have a situation where we have no control over the URL being encoded or not.
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 3 2018

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

commit 7bbfbc895823702689fac078a255ae8b5b0f2069
Author: Pavel Feldman <pfeldman@chromium.org>
Date: Wed Jan 03 05:29:55 2018

Simplify UnguessableToken::ToString, don't wrap it in ().

This change migrates unguessable tokens' serialization format from
`(790345...265EE7)` to `7903415...265EE7`.

When unguessable token is used for instrumentation and tracing, it can be used
as a handle. Having non-URL characters such as punctuation in the serialized
form of the token limits its use and requires it to be escaped / post-processed.

If wrapping in () is not essential to the token, I'm suggesting that we drop it.

Bug:  778263 
Change-Id: Ib88c15c478f6dca89425f272e35893dd399f91fd
Reviewed-on: https://chromium-review.googlesource.com/845146
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Pavel Feldman <pfeldman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526627}
[modify] https://crrev.com/7bbfbc895823702689fac078a255ae8b5b0f2069/base/trace_event/memory_infra_background_whitelist.cc
[modify] https://crrev.com/7bbfbc895823702689fac078a255ae8b5b0f2069/base/unguessable_token.cc
[modify] https://crrev.com/7bbfbc895823702689fac078a255ae8b5b0f2069/base/unguessable_token.h
[modify] https://crrev.com/7bbfbc895823702689fac078a255ae8b5b0f2069/base/unguessable_token_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment