core: added capability to pass cookies as CLI parameter #9170

RynatSibahatau · 2019-06-10T18:54:36Z

Summary

This feature adds the capability to pass cookies as a CLI parameter.

The current proposed approach to use "--extra-headers" parameter for setting cookies has a drawback for a wide range of enterprise applications that operates with the cookies (ex. authentication, localization, etc.).
Any Set-Cookie response from backend overwrites the Cookie header from extra-headers.
It introduces extra complexity of using lighthouse on CI in pure CLI mode (i.e without Puppeteer).
The fix intended to remove this complexity.

lighthouse http://localhost/ --extra-headers "{\"Cookie\":\"test=true\"}"
Request header: Cookie: test=true
Response header: Set-Cookie: locale=en;
All subsequent requests headers:
Actual request headers : Cookie: locale=en
Expected request headers: Cookie: test=true; locale=en

After merging this PR the following command introduced expected behaviour for cookies:

lighthouse http://localhost/ --extra-cookies "[{\"name\":\"test\", \"value\": \"true\", \"url\": \"http://localhost\"}]"

Links:
#6207 (comment)

Related Issues/PRs

fixes #6207

patrickhulce

Thanks very much for picking this up @RynatSibahatau! I left some comments but before we tackle those...

@brendankenny @hoten any thoughts or major changes you would want to propose for this? Linked issue has a tad more discussion but I know you've been playing around with some auth techniques so wanted to get your say on this before going any further.

lighthouse-cli/cli-flags.js

patrickhulce · 2019-06-10T19:14:16Z

lighthouse-cli/test/cli/bin-test.js

+ });
+
+ it('should read extra cookies from file', async () => {
+ const headersFile = require.resolve('../fixtures/extra-headers/valid.json');


seems like you mean to use the cookies one instead?

Fixed by f4e0a49

patrickhulce · 2019-06-10T19:18:28Z

lighthouse-cli/test/fixtures/static-server.js

@@ -105,6 +105,20 @@ function requestHandler(request, response) {
 }
 }

+ if (params.has('extra_cookie')) {


this file is for our smoketests, we'll want to make sure we add a smoketest that exercises this functionality which is probably going to be tricky...

maybe hold off on this step until we get buy-in from rest of LH team for this feature to work this way?

any reason to keep the changes to this file?

oh @RynatSibahatau think you could add a smoketest exercising this and our cookies logic?

oh @RynatSibahatau think you could add a smoketest exercising this and our cookies logic?

isn't that going to be kind of hard, @patrickhulce? What would we be testing, exactly?

I was thinking a page has a link or something that contains the contents of the cookies and it gets picked up by the anchor elements artifact we can assert

lighthouse-core/test/gather/driver-test.js

patrickhulce · 2019-06-10T19:28:18Z

lighthouse-core/gather/driver.js

+ return;
+ }
+
+ return this.sendCommand('Network.setCookies', {cookies});


AFAIK when the domain/url/path isn't set it will just use the existing target, because we're running this before we've navigated to the page I'm guessing the common case won't actually send the headers

good catch.

Are we going to entirely defer to the cookie definition for when they're included, e.g. for redirects to different origins, third-party iframes, etc? That format will definitely need to be documented, and it seems like it's going to be a huge pain trying to debug when something goes wrong, but I guess maybe folks are already used to that with other tools doing similar things?

What does WebPageTest do for setting cookies?

As far as I remember it is executed on "about:blank" page. This is why it is not really set anything, if not specifying either domain or URL. This another +1 for the bigger JSON structure.

Are we going to entirely defer to the cookie definition for when they're included, e.g. for redirects to different origins, third-party iframes, etc? That format will definitely need to be documented, and it seems like it's going to be a huge pain trying to debug when something goes wrong, but I guess maybe folks are already used to that with other tools doing similar things?

What does WebPageTest do for setting cookies?

I would assume we should rely on browser behavior for the session. A cookie is just a can-opener to test page usability under the specific conditions:

authorize access to content

allow access to CDN images

local show locale-specific content

perform sticky session

canary routing for CI

I don't think LH is intended to use like E2E test which might need any specific cookie handling between the tests.

@RynatSibahatau WDYT about asserting that we have either a url or domain on all the cookies here?

patrickhulce · 2019-06-10T19:29:36Z

lighthouse-cli/bin.js

@@ -122,6 +122,21 @@ async function begin() {
 cliFlags.extraHeaders = JSON.parse(extraHeadersStr);
 }

+ if (cliFlags.extraCookies) {
+ // TODO: LH.Flags.extraCookies is actually a string at this point, but needs to be


major bummer that we'll have a second one of these before we fix the first, this will also actually run into problems with the config-based CLI flags approach.

maybe we could at least extract this to a function to share between the two?

major bummer that we'll have a second one of these before we fix the first, this will also actually run into problems with the config-based CLI flags approach.

Yes, definitely, this shouldn't be done a second time :) The TODO even lists how to fix:

either the CLI flag or the setting should have a different name.

well that's the partial fix for the path problem, unless I'm misunderstanding?

there's not really an obvious fix I see for the "this thing is a complex object that can't be represented by yargs that we want to JSON.parse from the CLI but just use from node", --cookies-json-string I guess but that feels 🤢

@brendankenny @patrickhulce I am not pretty sure which fix is required here, as I am Java developer. I assume yargs can't provide you correctly typed JSON value out of the box.

You're right on that point @RynatSibahatau! The best we can do and what the comment suggests is that the property name should just be different for the yargs string version.

@brendankenny thoughts on extraCookiesJsonString for yargs and extraCookies for regular? (or we nix the extra prefix entirely I think was your last suggestion?)

bump on this @brendankenny

we could do --cookies for the flag (on the CliFlags interface) and extraCookies for the setting (on SharedFlagsSettings). Or --cookies-string and cookies.

Whatever it is, we should absolutely use this pattern. It's awkward, but most folks won't really care what the flag is (shortish is probably better, though), and the TODOs that have propagated from this point can only be removed in the future by doing this, so might as well do it before landing :)

I vote --cookies-string and --cookies then! 👍

connorjclark · 2019-06-10T21:04:26Z

I'm afraid I don't understand why the cookies are an array. Could you explain that? (I'm not well-versed with cookies).
I'm curious if we want to support the "load as file" behavior here. Do we have any other flags that support the value -> fallback to path? Does your use case absolutely require this (could you just read the contents from disk before calling making the command?)
Is using a JSON format natural here? In JS, it makes sense, but do other tools that handle cookies commonly use JSON over the key=value <space> pairs?

Co-Authored-By: Patrick Hulce <patrick.hulce@gmail.com>

RynatSibahatau · 2019-06-10T21:43:15Z

I'm afraid I don't understand why the cookies are an array. Could you explain that? (I'm not well-versed with cookies).

[RS] It is an array of objects because a cookie is not just a key/value pairs, but also have a big set of parameters (https://github.com/chromedp/cdproto/blob/master/network/network.go#L734). In the underlying protocol, it is also array https://github.com/chromedp/cdproto/blob/master/network/network.go#L825
The pretty valid case to have same cookie name, but with different values for different paths (a signed cookie as an example here: https://cloudinary.com/blog/protecting_images_and_videos_via_cookie_based_authentication)

I'm curious if we want to support the "load as file" behavior here. Do we have any other flags that support the value -> fallback to path? Does your use case absolutely require this (could you just read the contents from disk before calling making the command?)

[RS] LH already has one for headers. Here content might be even bigger. I decided to keep the same behavior. My use case doesn't require this. I am going to pass it via the command line.

Is using a JSON format natural here? In JS, it makes sense, but do other tools that handle cookies commonly use JSON over the key=value <space> pairs?

[RS] As I mentioned in the first answer it is not a key/value, so if we want to cover all possible use cases it is better to follow with JSON. Also, it avoids extra massaging for passing it. SetHeaders also uses JSON. And JSON is natural for a node language, LH is written on.

connorjclark · 2019-06-10T21:58:21Z

Thanks for educating me @RynatSibahatau! LGTM.

brendankenny

the general approach seems useful, my big questions:

do we really want to tie ourselves to the protocol's format? It's not necessarily the best or most explicable. On the other hand, it offers a lot of flexibility.
can we use a different flag name? I dislike "extra headers" ("extra to what?" is a whole explanation that's not needed for 95% of use cases). --cookies seems fine to me, if others are ok with that

brendankenny · 2019-06-10T21:24:15Z

lighthouse-cli/bin.js

@@ -122,6 +122,21 @@ async function begin() {
 cliFlags.extraHeaders = JSON.parse(extraHeadersStr);
 }

+ if (cliFlags.extraCookies) {
+ // TODO: LH.Flags.extraCookies is actually a string at this point, but needs to be


major bummer that we'll have a second one of these before we fix the first, this will also actually run into problems with the config-based CLI flags approach.

Yes, definitely, this shouldn't be done a second time :) The TODO even lists how to fix:

either the CLI flag or the setting should have a different name.

brendankenny · 2019-06-10T21:29:50Z

lighthouse-cli/bin.js

+ let extraCookiesStr = /** @type {string} */ (cliFlags.extraCookies);
+ // If not a JSON array, assume it's a path to a JSON file.
+ if (extraCookiesStr.substr(0, 1) !== '[') {
+ extraCookiesStr = fs.readFileSync(extraCookiesStr, 'utf-8');


yeah, I agree with @hoten that the ambiguity in the overloading of the flag is not worth it (can use the new --cli-flags-path if want to read from file!), but I do see we have it for --extra-headers already.

It would be nice to not include it by default and then consider adding it in the future if there's demand

--cli-flags-path approach looks good for me, I would agree we can avoid this dualism here for cookies. Should I remove it for the headers as well as part of the PR?
If there are no other major concerns for merging the PR. I can start implementing this.

I would love to remove it from headers but that'd be a major breaking change. Added it to our new list for next major version :) #9180

I have removed the capability to load cookie from file with the same parameter
1677a37

brendankenny · 2019-06-10T21:35:07Z

lighthouse-core/gather/driver.js

+ return;
+ }
+
+ return this.sendCommand('Network.setCookies', {cookies});


good catch.

Are we going to entirely defer to the cookie definition for when they're included, e.g. for redirects to different origins, third-party iframes, etc? That format will definitely need to be documented, and it seems like it's going to be a huge pain trying to debug when something goes wrong, but I guess maybe folks are already used to that with other tools doing similar things?

What does WebPageTest do for setting cookies?

readme.md

brendankenny · 2019-06-10T22:04:53Z

What does WebPageTest do for setting cookies?

looks like it's

usage: setCookie	<path>	<value>
example: setCookie	http://www.aol.com	zip=20166
example: setCookie	http://www.aol.com	TestData = Test; expires = Sat,01-Jan-2000 00:00:00 GMT

<path> - Path where the cookie should be used/stored
<value> - Cookie value (if expiration information isn't included it will be stored as a session cookie)

https://sites.google.com/a/webpagetest.org/docs/using-webpagetest/scripting#TOC-setCookie

patrickhulce · 2019-06-10T22:07:29Z

I agree with @RynatSibahatau that the JSON format for setting cookies offers an insane upgrade to dev experience compared to cookie strings. They are the bane of frontend network experience and there's no reason for us to prolong that pain here. I think the default case needs to be a bit streamlined though, i.e. we default the domain to the domain of the target page instead of being a noop on about:blank.

brendankenny · 2019-06-10T22:14:54Z

the JSON format for setting cookies offers an insane upgrade to dev experience compared to cookie strings

hmm:

--extra-cookies "[{\\"name\\":\\"session_id\\",\\"value\\":\\"x-men\\" }]"

:P

patrickhulce · 2019-06-10T22:20:13Z

@brendankenny oh puh-lease it's not like any sufficiently complicated setup requiring cookies would actually involve manually typing the cookies JSON. conversely, for a sufficiently complicated cookies setup automatically crafting the cookies string is a giant headache

brendankenny · 2019-06-10T22:22:02Z

I agree with @RynatSibahatau that the JSON format for setting cookies offers an insane upgrade to dev experience compared to cookie strings. They are the bane of frontend network experience and there's no reason for us to prolong that pain here.

I can totally buy this and I'll defer to you on the point, I just wanted to bring up the existing alternatives (the devil a developer knows can often beat better ergonomics or whatever) and the fact that probably at no point was Network.setCookie designed to optimize said ergonomics :)

At the very least the url/path/domain interactions are confusing and probably only documented in the chrome internals.

I think the default case needs to be a bit streamlined though, i.e. we default the domain to the domain of the target page instead of being a noop on about:blank.

This comes back to the issue of what to do with redirects, iframes, etc. We'll have to be able to declare which one is the domain.

patrickhulce · 2019-06-10T22:33:41Z

probably only documented in the chrome internals.

All the properties other than URL AFAICT are just the same properties developers should already be familiar with and would try to set with regular cookie syntax anyhoo (and URL just defaults the other properties like secure and domain don't ask why they didn't default path too 😆 )

https://cs.chromium.org/chromium/src/content/browser/devtools/protocol/network_handler.cc?type=cs&sq=package:chromium&g=0&l=417

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie

This comes back to the issue of what to do with redirects, iframes, etc. We'll have to be able to declare which one is the domain.

Very true. All we have to go on at this point in execution though is the provided URL since we'll need to send that request with the cookie. I think we solve this by just documenting. Maybe we need to warn if there were cookies set but there was a redirect?

RynatSibahatau · 2019-06-10T22:38:24Z

lighthouse-cli/bin.js

+ }
+
+ cliFlags.extraCookies = JSON.parse(extraCookiesStr);
+ }


Suggested change

}

if (!Array.isArray(cliFlags.extraCookies)) {

throw new Error('Not a valid JSON array');

}

cliFlags.extraCookies.forEach(key => {

const cookie = cliFlags.extraCookies[key];

if (! cookie.url || ! cookie.domain) {

// Default it to the domain value

cookie.url = url;

}

});

}

I can suggest this extra check and defaulting for the URL to avoid no-op

this is a good idea @RynatSibahatau! I think we would want this in lighthouse-core/ though so node users get this defaulting benefit too

@patrickhulce Could you take a look 074eee1?

that's exactly what I was hoping for! thanks!

RynatSibahatau · 2019-06-11T03:58:47Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

Am I right that you will be able to fix this on your side manually on merge?
I belive emails from the commit below for recommended chages, breaks CLA consent validation:

commit 72c9272
Author: RynatSibahatau RynatSibahatau@users.noreply.github.com
Date: Mon Jun 10 17:11:20 2019 -0400

Apply suggestions from code review

Co-Authored-By: Patrick Hulce <patrick.hulce@gmail.com>

brendankenny · 2019-06-26T01:27:53Z

lighthouse-cli/test/fixtures/static-server.js

@@ -105,6 +105,20 @@ function requestHandler(request, response) {
 }
 }

+ if (params.has('extra_cookie')) {


any reason to keep the changes to this file?

brendankenny · 2019-06-26T01:34:19Z

lighthouse-cli/bin.js

@@ -122,6 +122,21 @@ async function begin() {
 cliFlags.extraHeaders = JSON.parse(extraHeadersStr);
 }

+ if (cliFlags.extraCookies) {
+ // TODO: LH.Flags.extraCookies is actually a string at this point, but needs to be


we could do --cookies for the flag (on the CliFlags interface) and extraCookies for the setting (on SharedFlagsSettings). Or --cookies-string and cookies.

Whatever it is, we should absolutely use this pattern. It's awkward, but most folks won't really care what the flag is (shortish is probably better, though), and the TODOs that have propagated from this point can only be removed in the future by doing this, so might as well do it before landing :)

mariuspahontu · 2019-07-26T06:31:15Z

Hey guys,

Do you have any updates on this PR (when it will be merged into master)?

I want to let you know that, regarding the functionality, I'm using a fork of @RynatSibahatau's branch for my project and the cookies are correctly set when using the "--extra-cookies" CLI parameter.

Thanks,
Marius

patrickhulce · 2019-07-29T15:12:22Z

The evidence in #6207 (comment) suggests that the --extra-headers={cookie: 'foo'} approach has the extra cookies just totally ignored if there's other cookies set rather than overriding the existing cookies which we believed to be the case at the beginning of this.

Perhaps this suggests not doing any new flag but instead parsing the cookie header and setting those cookies sort of like @brendankenny was suggesting?

kkdev163

Thanks for your works.

This pipeline has already continued for 2 months, I don't know how long we still need to wait, so I manually copied the code of @RynatSibahatau into my forked repo, and then I push a commit called 'manually merge pr #9170'. If this action is illegal, please tell me, I will delete my forked repo soon.

In my test, I found two bugs, I will comment below:

kkdev163 · 2019-07-31T08:25:34Z

lighthouse-cli/bin.js

+ // the conversion here, but long term either the CLI flag or the setting should have
+ // a different name.
+ // @ts-ignore
+ const extraCookiesStr = /** @type {string} */ (cliFlags.extraCookies);


Need handle JSON file, because the readme.md has this usecase.

let extraCookiesStr = /** @type {string} */ (cliFlags.extraCookies); // If not a JSON object, assume it's a path to a JSON file. if (extraCookiesStr.substr(0, 1) !== '{') { extraCookiesStr = fs.readFileSync(extraCookiesStr, 'utf-8'); }

kkdev163 · 2019-07-31T08:35:51Z

lighthouse-core/gather/gather-runner.js

+ }
+ extraCookies.forEach(cookie => {
+ if (!cookie.url || !cookie.domain) {
+ // Default cookie URL to to current URL, if neither domain nor url is specified


according to this comment, should conditions be ?

!cookie.url && !cookie.domain

connorjclark · 2019-08-16T22:42:24Z

Perhaps this suggests not doing any new flag but instead parsing the cookie header and setting those cookies sort of like @brendankenny was suggesting?

Can we make a decision on this? This PR, or special casing --extra-headers for cookies?

chasekaylee · 2019-09-17T20:18:16Z

Hey there,

Was just wondering if there was an update on when this would be merged into master?

Cheers,

Chase

paulirish · 2019-09-17T21:50:42Z

@chasekaylee check out these:

https://github.com/GoogleChrome/lighthouse/blob/master/docs/authenticated-pages.md
https://github.com/GoogleChrome/lighthouse/tree/master/docs/recipes/auth

my expectation is those resources are more useful than the feature in this PR.
but if i'm wrong please let me know.

chasekaylee · 2019-09-25T21:22:19Z

ahh very cool. thanks! @paulirish

koomar · 2019-10-15T19:00:46Z

Are we moving ahead with this feature? Thanks. Also, @paulirish these links don't work for me.

connorjclark · 2019-10-15T19:11:09Z

I updated the links @koomar.

dvarrazzo · 2019-11-21T18:39:40Z

@paulirish I think the --extra-cookies options is fundamental to allow lighthouse to be scripted via command line using whatever other tool. The approaches suggested in the links are way more cumbersome.

My 2p is that this MR should be merged.

pkpattabhiraman · 2020-05-16T18:09:58Z

Hey Team,

Wondering when we can get this merged. Please advise!

patrickhulce · 2020-05-17T17:31:45Z

This PR hasn't been updated in almost a full year after there was disagreement on the team about the path forward. Since then, Lighthouse CI and the puppeteer script option offer far greater flexibility and control over these types of situations for the programmatic CI use case and are more consistent with our overall guidance on how to handle authenticated pages than patchwork CLI options with questionable ergonomics.

I'm inclined to close this PR rather than resurrect it at this point.

pkpattabhiraman · 2020-05-17T19:34:43Z

This PR hasn't been updated in almost a full year after there was disagreement on the team about the path forward. Since then, Lighthouse CI and the puppeteer script option offer far greater flexibility and control over these types of situations for the programmatic CI use case and are more consistent with our overall guidance on how to handle authenticated pages than patchwork CLI options with questionable ergonomics.

I'm inclined to close this PR rather than resurrect it at this point.

Thanks for the rapid response, @patrickhulce. Some of us other options like Chrome-Launcher and not puppeteer. Ability to inject extra-cookies would really make Lighthouse a complete solution instead of being tied to be used "only" with puppeteer for such use
cases.

patrickhulce · 2020-05-17T22:28:25Z

Some of us other options like Chrome-Launcher and not puppeteer

That's fine, you don't have to use puppeteer. If you're already using chrome-launcher you could use something like cri instead to talk to Chrome if you don't like puppeteer's API.

The point is that our general solution to this class of problems has been moving in the direction of "setup Chrome however you'd like by talking to it" rather than patching various holes with more and more CLI flags. Now that we have a first-class answer to the CI use case, this feature is unlikely to receive the priority support being requested here.

RynatSibahatau · 2020-05-20T01:00:04Z

I don’t mind closing the PR as I don’t own the Lighthouse product and don’t know details of its roadmap and desired capabilities. I agree that Puppeteer is a more flexible solution. But be alerted that there was some back and forth on Puppeteer repo regarding how correctly set a cookie: puppeteer/puppeteer#1342

I think it is worth providing correct puppeteer script in Lighthouse documentation in regards to how to properly set a cookie.
Last time I worked with Puppeteer I also faced an issue with that that I had to navigate to the site before setting a cookie for the site. It means I have to put traffic on the live site from the lighthouse’s puppeteer before I can set a cookie to redirect it to a canary one. If it still the true, Puppeteer usage scenario still have some drawbacks.

For my use case (it was solved even before I have opened this PR), I was trying to use TEST cookie to reroute canary traffic to the canary version of the service during deployment. I achieved that by switching from cookie-based routing on k8s virtual service to header-based routing. Chrome plugins (like ModHeader) also allows me to specify this header for manual testing of canary version. So, I don’t see any benefit for my specific case to use cookie vs header.

Thank you for mentioning this issue on the documentation page!

If code owners decide it is good to have CLI parameter, you are welcome to start from the current state of PR. I also can finish it in case there is a consensus within code owners.

patrickhulce · 2020-05-20T01:46:04Z

Thanks for the additional context @RynatSibahatau and for all your great work on the PR thusfar! ❤️

I think it is worth providing correct puppeteer script in Lighthouse documentation in regards to how to properly set a cookie.

This is a great idea 👍 Regardless of the outcome here we should do this.

Last time I worked with Puppeteer I also faced an issue with that that I had to navigate to the site before setting a cookie for the site. It means I have to put traffic on the live site from the lighthouse’s puppeteer before I can set a cookie to redirect it to a canary one. If it still the true, Puppeteer usage scenario still have some drawbacks.

If that's true, that would unfortunately plague this approach as well. The PR as-is would set the cookie before navigation.

patrickhulce · 2020-09-21T21:58:13Z

I think it is worth providing correct puppeteer script in Lighthouse documentation in regards to how to properly set a cookie.

I'm going to follow up with this solution and we'll close this PR for now. Thank you very much for your work here @RynatSibahatau we do appreciate it and #11313 should make this sort of thing even easier in the future as well.

Initial implementation

421d262

googlebot added the cla: yes label Jun 10, 2019

vercel bot deployed to staging June 10, 2019 18:54 View deployment

patrickhulce reviewed Jun 10, 2019

View reviewed changes

Fix build issues

1ea7f7d

vercel bot deployed to staging June 10, 2019 21:00 View deployment

Apply suggestions from code review

72c9272

Co-Authored-By: Patrick Hulce <patrick.hulce@gmail.com>

googlebot added cla: no and removed cla: yes labels Jun 10, 2019

vercel bot deployed to staging June 10, 2019 21:11 View deployment

RynatSibahatau added 2 commits June 10, 2019 17:12

Fix copy&paste issue with fixture name

f4e0a49

Merge branch 'master' of github.com:RynatSibahatau/lighthouse

8de0262

vercel bot deployed to staging June 10, 2019 21:12 View deployment

brendankenny reviewed Jun 10, 2019

View reviewed changes

Fix build: fix type check

2e1365b

vercel bot deployed to staging June 10, 2019 22:21 View deployment

RynatSibahatau commented Jun 10, 2019

View reviewed changes

RynatSibahatau marked this pull request as ready for review June 11, 2019 03:02

RynatSibahatau requested a review from paulirish as a code owner June 11, 2019 03:02

patrickhulce mentioned this pull request Jun 11, 2019

Unable to set cookies when they are added as "--extra-headers" #9178

Closed

brendankenny requested changes Jun 26, 2019

View reviewed changes

brendankenny removed the cla: no label Jul 9, 2019

GoogleChrome deleted a comment from googlebot Jul 9, 2019

brendankenny added the waiting4committer label Jul 9, 2019

patrickhulce mentioned this pull request Jul 29, 2019

set-cookie seems to override the cookie header instead of appending the new cookies #6207

Closed

kkdev163 pushed a commit to kkdev163/lighthouse that referenced this pull request Jul 31, 2019

manually merge pr GoogleChrome#9170

97ec8d6

kkdev163 pushed a commit to kkdev163/lighthouse that referenced this pull request Jul 31, 2019

feat: fix the bug of pr GoogleChrome#9170

b22bce2

kkdev163 suggested changes Jul 31, 2019

View reviewed changes

connorjclark mentioned this pull request Aug 16, 2019

Question: Auth #9569

Closed

patrickhulce mentioned this pull request Nov 12, 2019

Ability to set cookies GoogleChrome/lighthouse-ci#58

Closed

paulirish mentioned this pull request Jan 28, 2020

docs: tweaks to auth + pptr docs #10277

Merged

patrickhulce closed this Sep 21, 2020

This was referenced Sep 21, 2020

Add page.setCookie documentation #11470

Closed

docs(auth): add setCookie example #11473

Merged

- }
+ if (!Array.isArray(cliFlags.extraCookies)) {
+ throw new Error('Not a valid JSON array');
+ }
+ cliFlags.extraCookies.forEach(key => {
+ const cookie = cliFlags.extraCookies[key];
+ if (! cookie.url || ! cookie.domain) {
+ // Default it to the domain value
+ cookie.url = url;
+ }
+ });
+ }

core: added capability to pass cookies as CLI parameter #9170

core: added capability to pass cookies as CLI parameter #9170

Conversation

RynatSibahatau commented Jun 10, 2019 • edited by brendankenny Loading

patrickhulce left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

connorjclark commented Jun 10, 2019

RynatSibahatau commented Jun 10, 2019 • edited Loading

connorjclark commented Jun 10, 2019

brendankenny left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendankenny commented Jun 10, 2019

patrickhulce commented Jun 10, 2019

brendankenny commented Jun 10, 2019

patrickhulce commented Jun 10, 2019 • edited Loading

brendankenny commented Jun 10, 2019

patrickhulce commented Jun 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RynatSibahatau commented Jun 11, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mariuspahontu commented Jul 26, 2019 • edited Loading

patrickhulce commented Jul 29, 2019

kkdev163 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkdev163 Jul 31, 2019 • edited Loading

Choose a reason for hiding this comment

connorjclark commented Aug 16, 2019

chasekaylee commented Sep 17, 2019

paulirish commented Sep 17, 2019 • edited by connorjclark Loading

chasekaylee commented Sep 25, 2019

koomar commented Oct 15, 2019 • edited by connorjclark Loading

connorjclark commented Oct 15, 2019

dvarrazzo commented Nov 21, 2019

pkpattabhiraman commented May 16, 2020

patrickhulce commented May 17, 2020

pkpattabhiraman commented May 17, 2020

patrickhulce commented May 17, 2020

RynatSibahatau commented May 20, 2020

patrickhulce commented May 20, 2020

patrickhulce commented Sep 21, 2020

RynatSibahatau commented Jun 10, 2019 •

edited by brendankenny

Loading

RynatSibahatau commented Jun 10, 2019 •

edited

Loading

patrickhulce commented Jun 10, 2019 •

edited

Loading

mariuspahontu commented Jul 26, 2019 •

edited

Loading

kkdev163 left a comment •

edited

Loading

kkdev163 Jul 31, 2019 •

edited

Loading

paulirish commented Sep 17, 2019 •

edited by connorjclark

Loading

koomar commented Oct 15, 2019 •

edited by connorjclark

Loading