Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DevTools SetCookie support #3218

Closed
mitchcapper opened this issue Aug 19, 2020 · 19 comments
Closed

DevTools SetCookie support #3218

mitchcapper opened this issue Aug 19, 2020 · 19 comments

Comments

@mitchcapper
Copy link
Contributor

the cef SetCookie support is limited not supporting things like SameSite settings. DevTools does not have this problem.

Do we want to start a dev tools extension class? I can submit a PR but here is setCookie working:

public enum CookieSameSite { None, Strict, Lax }

		public async Task<bool> SetCookie(String name, String value, String domain, long? expires = null, bool secure = false, bool httpOnly = false, String path = "", CookieSameSite same_site = CookieSameSite.None) {
			var res = await browser.ExecuteDevToolsMethodAsync(0, "Network.setCookie", new Dictionary<string, object> {
				{ "name", name}, { "value", value}, { "domain",domain}, { "expires", expires },
				{ "secure", secure },{ "httpOnly", httpOnly },{ "path",path ?? ""},{"sameSite",same_site.ToString()}
			});
			return res != 0;
		}
		public Task<bool> SetCookie(String name, String value, String domain, DateTimeOffset? expires = null, bool secure = false, bool httpOnly = false, String path = "", CookieSameSite same_site = CookieSameSite.None) => SetCookie(name, value, domain, expires?.ToUnixTimeSeconds(), secure, httpOnly, path, same_site);
@amaitland
Copy link
Member

I created issue #3165 with the intention of writing a wrapper API around CEF DevTools methods, just not quite clear what that implementation looks like just yet, if you've got some suggestions input is welcome. I had a look at what other .Net DevTools protocol implentations with the plan on using Roslyn to generate the API wrappers based on the actual DevTools protocol definition, haven't made any meaningful progress yet.

By all means submit a PR, there is already https://github.com/cefsharp/CefSharp/blob/master/CefSharp/DevToolsExtensions.cs which your welcome to add to. Default builds still use vs2015 so you'll be limited in syntax that you can use for now.

@mitchcapper
Copy link
Contributor Author

It is true, we likely could write something that either parsed the native code to generate the devtool methods automatically or even did so from a documentation POV. Given the fact dev tool methods likely don't change that often it could be done as a one off to get the initial work done and then manually tweak them by hand to make them a bit more user friendly.

I was wondering if we wanted to do them all as extension methods or if we wanted to create a class to put them under. The syntax could be browser.DevToolMethods().SetCookie to avoid cluttering the main browser object. If we are trying to mimic the official devtools syntax we should probably stick to subclasses as well so browser.DevToolMethods().Network.SetCookie

granted if devtools renames something we would have to decide if we wanted to break back compat, use a placeholder function, or stick with the original name.

@amaitland
Copy link
Member

DevTools protocol is defined in two pdl files, there's a python script to convert them to JSON which can easily be parsed.

https://source.chromium.org/chromium/chromium/src/+/master:third_party/inspector_protocol/convert_protocol_to_json.py

It looks like we might be able to get the JSON definitions from https://github.com/ChromeDevTools/devtools-protocol/tree/master/json

The finer details like how the different DevTools domains (page, network, etc) are represented isn't clear. I've been meaning to have another look at puppeteer/puppeteer-Sharp determine if it makes sense to model our API off an established standard.

As part of #3165 I plan to add a DevToolsClient class that will represent a single IBrowser instance, there's some object lifecycle management that we need to consider.

Having a tool that can be run manually on the rare occasion the API changes is what I was planning.

@amaitland
Copy link
Member

Two existing .Net projects I've looked at briefly.

https://github.com/BaristaLabs/chrome-dev-tools-generator
https://github.com/MasterDevs/ChromeDevTools

@mitchcapper
Copy link
Contributor Author

Do you have a timeframe on auto-generating the classes? I am fine submitting a PR so we could do manual wrappers for things in the mean time. I have already done some of the other storage classes could add user agent, and clearing cookies/cache/storage as well.

@amaitland
Copy link
Member

I'm hoping to have the generated classes done for the 85 release.

A PR with some manually written extension methods would be welcome 👍 I can include them in the upcoming 84 release.

@amaitland
Copy link
Member

Looking at the example provided in a little more detail

public async Task<bool> SetCookie(String name, String value, String domain, long? expires = null, bool secure = false, bool httpOnly = false, String path = "", CookieSameSite same_site = CookieSameSite.None)
{
	var res = await browser.ExecuteDevToolsMethodAsync(0, "Network.setCookie", new Dictionary<string, object>
	{
		{ "name", name}, { "value", value}, { "domain",domain}, { "expires", expires },
		{ "secure", secure },{ "httpOnly", httpOnly },{ "path",path ?? ""},{"sameSite",same_site.ToString()}
	});
	return res != 0;
}

The ExecuteDevToolsMethodAsync currently returns the message Id. The res != 0 check will only determine if a message Id was returned at not the success of setting the cookie.

Looking at the Network.setCookie should return json response containing a success (bool). To determine if the cookie was set successfully we'll need to add an observer to get the results.

I'll look at adding a generic observer that can be used to obtain command results initially.

@mitchcapper
Copy link
Contributor Author

Yes essentially the commands I have done thus far only return false if the dev tools message can't be sent (as it returns 0 in this case. Obviously getting data out of dev tools is a plus.

I can submit up the manual extensions without an issue, mainly I don't want to cause an issue for you in terms of maintainability down the line when you do automate.

For one, as mentioned, if we want to do them all at a flat level right now, or if I should emulate the depth of the real commands. If there is a DevToolsClient (and maybe a browser.GetDevToolsClient() or similar) it would then be DevToolsClient.Network.setCookie. I would assume if hoping to move to an automated framework generation trying to have methods that stick to the official protocol and parameters is best (with the potential of additional overloads or similar to simplify things like with the cookie TimeSpan version). I am also not sure the best way to handle things like custom types. I could do the official signature that just takes a string, and then an overload with a custom enum like I did for CookieSameSite.

Some of the other ones I assume are decently useful could do extensions for

Network.clearBrowserCache
Network.clearBrowserCookies
Storage.clearDataForOrigin
Emulation.setUserAgentOverride
Emulation.setEmitTouchEventsForMouse
Emulation.setGeolocationOverride
Emulation.setTimezoneOverride
Emulation.setVisibleSize

@amaitland
Copy link
Member

es essentially the commands I have done thus far only return false if the dev tools message can't be sent (as it returns 0 in this case. Obviously getting data out of dev tools is a plus.

I have a rough DevToolsClient class, which usage wise looks something like https://github.com/amaitland/CefSharp/blob/d535d4c04b096f9483dc7c51e4bc1a60cd10c0f3/CefSharp.Example/DevTools/DevToolsExtensions.cs#L36-L54

I can submit up the manual extensions without an issue, mainly I don't want to cause an issue for you in terms of maintainability down the line when you do automate.

Thanks 👍

it would then be DevToolsClient.Network.setCookie

I've seen this approach used and I think that makes sense, each class would have methods and events that match the DevTools protocol.

I am also not sure the best way to handle things like custom types. I could do the official signature that just takes a string, and then an overload with a custom enum like I did for CookieSameSite.

Not sure, let me have a think about it overnight and I'll get back to you.

@amaitland
Copy link
Member

amaitland commented Aug 27, 2020

Looking at https://github.com/ChromeDevTools/devtools-protocol/blob/master/json/browser_protocol.json and I think parsing and generating at least all the enum should be pretty trivial, I'll hack something together. I'm thinking it might be better to just generate everything save double handling. I'll hack something together hopefully in the next few days.

@amaitland amaitland reopened this Aug 27, 2020
@amaitland
Copy link
Member

I have the start of a parser/generator at https://gist.github.com/amaitland/721cbf556b3c6cf53434c8ea2944b711

It's not pretty as I'm not totally fluent in Roslyn. I'll hopefully have it generating methods in the next few days.

@amaitland
Copy link
Member

amaitland commented Sep 1, 2020

See amaitland@551f1aa for the first draft of the generated classes. It compiles and some of the simpler methods probably execute ok. Complex ones there's still some work to do.

TODO:

  • Map complex params/enums
  • Strongly typed return types (only return the generic DevToolsMethodResult at the moment).

@amaitland
Copy link
Member

I've created #3229 which has my work so far. It's not too far from being usable, simple methods work as expected, I've only generated non experimental methods, I need to workout which version of the DevTools API we should be referencing (generating against the master branch currently which isn't going to work for a stable release).

A simple example can be seen at https://github.com/cefsharp/CefSharp/pull/3229/files#diff-aa6ec4729bc06f598f7197f4bc89ff05R80

Once methods are working as expected then I'll look at adding support for strongly typed events.

Comments/feedback welcome. As always naming could use some work.

@mitchcapper
Copy link
Contributor Author

The generated code is certainly verbose, but I guess there is no problem with that. As for which version: would you tie is simply to the branch that is the latest CEFSharp is built off of? Someone trying to use an older CEF release could just pull an older cefsharp tag.

@amaitland
Copy link
Member

The generated code is certainly verbose, but I guess there is no problem with that

I used SyntaxGenerator for code generation, it was much easier to get started with, a little limited when compared to SyntaxFactory, much easier to work with.

As for which version: would you tie is simply to the branch that is the latest CEFSharp is built off of? Someone trying to use an older CEF release could just pull an older cefsharp tag.

As I'm trying to avoid generating the json files from the pdl files. It's a matter of matching a chromium release branch to the correct revision from https://github.com/ChromeDevTools/devtools-protocol/commits/master/json

I don't believe revisions change each version so I'm hoping to have a unit test that can let me know when we get out of sync and need to regenerate.

@amaitland
Copy link
Member

Also, if you've set --remote-debugging-port=9222 with Chrome, the complete protocol version it speaks is available at localhost:9222/json/protocol.

As per https://chromedevtools.github.io/devtools-protocol/#json it appears we can get the actual protocol out of the browser itself, can use CefSharp.OffScreen to self generate effectively.

@amaitland
Copy link
Member

Generating based on the protocol json provided by the browser works quite nicely.

  • Only methods are supported, not events yet.
  • Experimental methods are now being generated
  • Deprecated method aren't generated.

@mitchcapper Do you have any example code you'd like turned into Unit Tests? A little bit more testing and I'll probably merge the changes.

@helpr helpr bot added pr-merged and removed pr-available labels Sep 21, 2020
@amaitland
Copy link
Member

I've merged #3229

I'll add an overload for SetCookie that takes a DateTimeOffset, any other helpful overloads you'd like?

@amaitland
Copy link
Member

Adding an overload for SetCookieAsync with DateTime/DateTimeOffset is actually rather awkward to use due to the bulk number of option params.

If you don't want to specify an expiry you end up having to cast (long?)null just to pick an overload.

There's also the complication that ToUnixTimeSeconds wasn't added until .Net 4.6, so we'd have to write a .Net 4.5.2 version.

So I won't add any overloads for now.

Example of setting expiry.

using (var devToolsClient = browser.GetDevToolsClient())
{
	var response = await devToolsClient.Network.SetCookieAsync(name, value, domain: domain, sameSite: sameSite);
	var expiry = DateTimeOffset.UtcNow.AddDays(10);
	var response = await devToolsClient.Network.SetCookieAsync(name, value, domain: domain, sameSite: sameSite, expires: expiry.ToUnixTimeSeconds());
}

UTC time in seconds, counted from January 1, 1970.

As per https://chromedevtools.github.io/devtools-protocol/tot/Network/#type-TimeSinceEpoch the expiry is in UTC


Potentially could create a TimeSinceEpoch class and use a strongly typed param instead of long?. Will defer that until a little later. If you feel that's important then please feel free to open a new feature request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants