New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 731618 link

Starred by 31 users

Issue metadata

Status: Started
Owner:
OOO until 4th
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug


Show other hotlists

Hotlists containing this issue:
Hotlist-1


Sign in to add a comment

Autocomplete of relative URLs fails when using basic authentication

Reported by m.bey...@walz.de, Jun 9 2017

Issue description

Chrome Version       : 59.0.3071.86
URLs (if applicable) : http://example:example@domain.com
Other browsers tested:
  Add OK or FAIL, along with the version, after other browsers where you
have tested this issue:
    Firefox: OK 53.0.3 (32bit)

What steps will reproduce the problem?
(1) Given a website with basic authentication and relative URLs for CSS Style Sheets
(2) Visit this Website with Basic Authentication, typing the credentials into the url e.g. "http://example:example@domain.com/"

What is the expected result?
- The CSS is correclty loading via http://domain.com/style.css, Chrome extends the relative path without credentials, which are not allowed any more

What happens instead?
- Chrome completes the relative path of the css file "/styles.css" including the authentication "http://example:example@domain.com/style.css" and gives a warning the console "[Deprecation] Subresource requests whose URLs contain embedded credentials ..."


Please provide any additional information below. Attach a screenshot if
possible.

Problem occured due this feature
https://www.chromestatus.com/feature/5669008342777856

In my opinion it doesn't make sense, that Chrome completes URLs with credential information that is not allowed by itself. It should just complete them without credentials and everything is working fine.
 
Cc: tsepez@chromium.org pbomm...@chromium.org abdulsyed@chromium.org mkwst@chromium.org
Components: Blink>Loader
Labels: pre-stable-59.0.3071.86 M-59 OS-Windows

Comment 2 by mkwst@chromium.org, Jun 12 2017

Cc: -mkwst@chromium.org
Owner: mkwst@chromium.org
Status: Started (was: Unconfirmed)
Yeah, that's unintended. We should carve out same-origin matches with matching usernames/passwords in order to ensure that we're not breaking this use case yet.

(That said, given that the number of top-level navigations to URLs with embedded credentials is hovering around 0%, we're considering removing this functionality entirely... See discussion in https://bugs.chromium.org/p/chromium/issues/detail?id=504300#c13)

Comment 4 by mkwst@chromium.org, Jun 12 2017

Cc: mkwst@chromium.org
 Issue 732334  has been merged into this issue.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 13 2017

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

commit 0ae6b276afc7f7fb1a5adec900717ce03cb83eaf
Author: Mike West <mkwst@chromium.org>
Date: Tue Jun 13 21:01:27 2017

Allow embedded credentials for relative URLs.

If a user visits `http://user:pass@example.com/`, we should allow
`<img src='/whatever.png'>`. Currently, we're doing the embedded
credential check on the completed URL, which ends up blocking the
request.

Bug: 731618
Change-Id: I5a15187a2a0fd39822467d64efeedaae637765a8
Reviewed-on: https://chromium-review.googlesource.com/530308
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479147}
[modify] https://crrev.com/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[rename] https://crrev.com/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf/third_party/WebKit/LayoutTests/external/wpt/fetch/security/dangling-markup-mitigation.tentative.html
[add] https://crrev.com/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf/third_party/WebKit/LayoutTests/external/wpt/fetch/security/embedded-credentials.tentative.sub.html
[add] https://crrev.com/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf/third_party/WebKit/LayoutTests/external/wpt/fetch/security/support/embedded-credential-window.sub.html
[modify] https://crrev.com/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf/third_party/WebKit/Source/core/loader/BaseFetchContext.cpp
[modify] https://crrev.com/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf/third_party/WebKit/Source/core/loader/BaseFetchContext.h
[modify] https://crrev.com/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf/third_party/WebKit/Source/core/loader/BaseFetchContextTest.cpp
[modify] https://crrev.com/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf/third_party/WebKit/Source/core/loader/FrameFetchContext.h
[modify] https://crrev.com/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp
[modify] https://crrev.com/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf/third_party/WebKit/Source/core/loader/WorkerFetchContext.h
[modify] https://crrev.com/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 14 2017

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

commit a9bca94033a6d13e17d5d7dec1e9ce9625f90772
Author: Mike West <mkwst@chromium.org>
Date: Wed Jun 14 20:01:58 2017

Restore the 'BlockCredentialedSubresources' runtime flag.

There's some negative feedback flowing in as the change hits stable, so.
Let's put the flag back for the time being.

Bug: 731618
Change-Id: If37bd732e9e2b364cd1eedeedbb85838602453d3
Reviewed-on: https://chromium-review.googlesource.com/535521
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479478}
[modify] https://crrev.com/a9bca94033a6d13e17d5d7dec1e9ce9625f90772/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/a9bca94033a6d13e17d5d7dec1e9ce9625f90772/third_party/WebKit/Source/core/loader/WorkerFetchContext.cpp
[modify] https://crrev.com/a9bca94033a6d13e17d5d7dec1e9ce9625f90772/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5

Comment 7 by phistuck@gmail.com, Jun 29 2017

Will this be merged to some release (59? 60?)?
This lead to crashing all my test cases because of the broken CSS. When can we expect a fix?

Comment 9 by m.bey...@walz.de, Jun 29 2017

Until the fix is released, you can use the switch

--disable-blink-features=BlockCredentialedSubresources

to run your tests.

Comment 10 by mkwst@chromium.org, Jun 29 2017

Labels: Merge-Request-60
Let's get it back to 60. I think the ship has sailed for 59 (and the overall numbers of top-level navigation to credentialed URLs is very low).

Hello, friendly release managers: WDYT about merging back the two patches in this bug (https://chromium.googlesource.com/chromium/src.git/+/0ae6b276afc7f7fb1a5adec900717ce03cb83eaf and https://chromium.googlesource.com/chromium/src.git/+/a9bca94033a6d13e17d5d7dec1e9ce9625f90772). They've baked on dev channel for a week, and seem safe to merge back.
Project Member

Comment 11 by sheriffbot@chromium.org, Jun 29 2017

Labels: Merge-Reject-60 Hotlist-Merge-Reject
The bug is marked as P3 or Feature. It should not be merged as M60 is in beta. 
Please contact the approriate milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 12 by mkwst@chromium.org, Jun 29 2017

Cc: bustamante@chromium.org amineer@chromium.org
Labels: OS-Android OS-Chrome OS-Linux OS-Mac
+bustamante@/amineer@: I agree with the bot's opinion in general! But I'd still like to merge this back to 60. It solves a problem that developers are seeing frequently enough in the wild to find threads related to this bug and comment on them. I'd appreciate y'all taking a slightly less automated look at this. :)

Comment 13 by mkwst@chromium.org, Jun 30 2017

Labels: -Pri-3 -Merge-Reject-60 Merge-Request-60 Pri-2
Poking at flags in the hopes of catching folks before they disappear for the holiday. :)
Project Member

Comment 14 by sheriffbot@chromium.org, Jun 30 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: M60 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-60 Merge-Rejected-59
Since we're past M60 merge point, our bar is very high for taking in merges. Based on the description and comments, this doesn't appear to be a critical bug. Can we wait until M61? What are the downsides if we wait until then?

Our goal right now is to stabilize M60 branch and only take critical bug regressions, stable blockers, and security bugs. 
Cc: kochi@chromium.org
 Issue 738408  has been merged into this issue.
abdulsyed@: In short, this bug prevents folks who navigate to URLs like `https://user:pass@whatever.com/` from loading subresources with relative URLs (like `<img src="/img.png">`). I agree with your underlying suggestion that this doesn't happen terribly often, but it does seem to be more concentrated in a population of developers who rely on it for various internal usage.

There is a reasonable workaround for a developer population (the `--disable-blink-features=BlockCredentialedSubresources` flag noted above), but that's a difficult workaround to use on non-desktop platforms. Given the volume of feedback, it seems to me that this is something worth addressing.

Would y'all be more comfortable with merging a patch back that added that flag to `about:flags` to make it universally accessible?
 Issue 738917  has been merged into this issue.

Comment 19 Deleted

Please add this as an about:flag so that it can easily be dealt with across platforms and user experience levels.

Also, couldn't have this just been a warning, instead of this outright block? 

Seems like a little more thought into how you treat your users in an error scenario and you wouldn't be ruining so many peoples day with this change. I feel like this happened recently with another recent change(https).

Prefer the use of soft errors over hard.

Comment 21 by mkwst@chromium.org, Jul 12 2017

abdulsyed@: Ping?

Comment 22 by mkwst@chromium.org, Jul 12 2017

 Issue 741573  has been merged into this issue.
Chatted with mkwst@ offline - since stable release is less than two weeks away, we agreed that it's best to target fix for M61. Risk of taking a large patch is higher this late, and can have negative consequences on a broader scale. Apologies for the inconvenience that this is causing and please use the current workaround, mentioned in comment 9, for desktop fix. Fix will be available soon in M61. 
 Issue 745342  has been merged into this issue.
I agree with reporter.
simply do not transfer the "user:pass" part from the main url provided in the address bar ... so sub-resources loading will be attempted and will succeed, as the "Authorization" HTTP Header is set ...

Yet, I can understand why you are "blocking" the urls when they are hard-coded "user:pass" in the html, to avoid CSRF attempts, but realistically even the "Authorization" HTTP Header could lead to a CSRF exploit


Comment 26 by mkwst@chromium.org, Jul 19 2017

 Issue 746131  has been merged into this issue.

Comment 27 by mkwst@chromium.org, Jul 20 2017

 Issue 746773  has been merged into this issue.
I am having issues with my tests failing due to longer being able to pass the username and pw into URL for basic authentication pop up...
I tried to add to my config.js file the following. But did not work, pop up still appears.

chromeOptions: {
 args:['--disable-blink-features=BlockCredentialedSubresources']
},
Please try suggestion in comment 9 for a workaround for desktop. 
Looks like this is working as before in Chrome Version 61.0.3163.79 (Official Build) (64-bit)

I am using it on Windows 7

Comment 31 by fur...@gmail.com, Sep 6 2017

I can confirm this is also fixed for me on Windows 10 with Chrome 61.
Not working for me on Windows 7 x64 with Chrome Version 61.0.3163.79 (Official Build) (64-bit)

I still have to use Chrome Portable Version 58.0.3029.110 (64-bit) for pages with basic authentication to work (all webcams in AV enabled rooms across our organization=university.)
#32 - that is weird. Though you might have a different case. I suggest filing a new issue for it with as much details as possible, a test URL that reproduces the issue would be help a great deal. You can comment here with the issue number.
Thank you guys for fixing this in 61. Eagerly waited for this update.
Cc: -amineer@chromium.org
No longer on the Chrome team, e-mail me @google.com if any attention still required from me here, otherwise good luck!

Sign in to add a comment