-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
Avoid using lookaheads to support PCRE in Caddy/Go #36
Comments
@oprypkhantc Hi. Edge on Chromium has "Edg" in user agent |
@oprypkhantc Explore user agent strings examples you can here: https://explore.whatismybrowser.com/useragents/explore/software_name/edge/ |
Early versions of Edge browser were based on own EdgeHTML engine, but user agent of these versions include "Chrome" substring. It was made to "support" websites what requires Chrome browser (or something like that), but in fact these engines are not compatible. So we need to differentiate user agents like this. Same thing for Mobile IE (i guess from windows phone) and Android I guess you are receiving this regexp from browserslist-useragent-regexp. If your browserslist query will not request Chrome <= 70 you will not receive regexp with lookahead |
Somehow I looked at modern user agents of Edge and saw "Edge" at the end of it, when obviously there was none. Sorry 🤦🏻 And yes, you're correct, I'm getting the regex from
What's also important is that Maybe you have an insights on how to properly solve this? |
@oprypkhantc We can try something like that:
but it risky because maybe there is some Chromium based browser that have some of this chars on that places You can research Chromium <= 70 based browsers and their user agents to fit this workaround. For example Brave doesn't add own name in ua, so it purely Chrome - this regexp should be ok |
Yes. Regexp on first screen is matched because you don't have end of line, you have new line char. On second screen regex is not matched because of minor version mismatch. By default exact version from browserslist are embedded to regex. You can try |
Thanks. With multiline flag and
This is extremely ugly, but should work the same way the negative lookahead works. It matches regular Chrome, Opera, new Edg, but not Edge. Would you accept a PR changing this (and likely other regexes too) to not use lookaheads/lookbehinds, and a test to make sure they're not added in the future? I understand that Go's limitations should not affect this project, but Caddy is quite nice other than this, and is even shipped by default in some cases (https://frankenphp.dev/), so it'd be nice to have this. |
@oprypkhantc If it will work correctly - sure, i will accept |
Would you like to work on this feature?
What problem are you trying to solve?
Hey.
Caddy (a web server, quite popular) is written with Go, which does not support lookaheads (and some other features) due to performance reasons. Unfortunately, there's no easy way to work this around.
The reason I care about this is because we're using Caddy's
header_regexp()
feature to match the User-Agent and redirect users to an "outdated browser" page, and this package uses a negative lookahead in two of it's regexp's:https://github.com/TrigenSoftware/ua-regexes-lite/blob/main/index.js#L72
https://github.com/TrigenSoftware/ua-regexes-lite/blob/main/index.js#L186
I would like to remake those to not use lookaheads, but I don't get the point of them and tests for them do not use real user agents, so they are completely useless:
https://github.com/TrigenSoftware/ua-regexes-lite/blob/main/test/useragents.js#L95
Notice the
Edg
notEdge
at the end of the user agent string; if I actually use the correct user agent withEdge
, tests start to fail. So it seems like this regex has never worked properly anyway?Describe the solution you'd like
Not use lookaheads, as that doesn't seem to be necessary.
Describe alternatives you've considered
Currently I'm doing this:
sed -e 's#\.\*Safari\\/(?!\[\\d\.\]+ Edge\\/\[\\d\.\]+$)##g'
, which replaces the negative lookahead part. But of course I'd love to avoid these kind of hacks that can break at any moment.Documentation, Adoption, Migration Strategy
No response
The text was updated successfully, but these errors were encountered: