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

SyntaxError: Invalid regular expression: invalid group specifier name #105

Closed
muuvmuuv opened this issue Feb 3, 2020 · 36 comments
Closed

Comments

@muuvmuuv
Copy link

muuvmuuv commented Feb 3, 2020

Hey, I'm getting this error here on iOS and macOS Safari:

SyntaxError: Invalid regular expression: invalid group specifier name

It comes from here:

// Remove duplicate slashes if not preceded by a protocol
if (urlObj.pathname) {
  urlObj.pathname = urlObj.pathname.replace(/(?<!https?:)\/{2,}/g, '/');
}

My code: https://github.com/muuvmuuv/portfolio/blob/master/src/elements/Link.jsx
Live error: https://portfolio-git-development.muuvmuuv.now.sh/

I am using version 5.x

@TrySound
Copy link
Contributor

TrySound commented Feb 3, 2020

@muuvmuuv Look here for details
76172da#diff-168726dbe96b3ce427e7fedce31bb0bcR120

@muuvmuuv
Copy link
Author

muuvmuuv commented Feb 3, 2020

Oh... what a shame. Thought it says >4 not =4 but okay will fix the version tho.

Can we leave this open until browsers will add support for negative lookbehinds?

Browser support:

@kornysietsma
Copy link

Note that this also impacts Gatsby projects in "develop" mode on firefox 74 (current) - negative lookbehinds work for building a site, but when developing it also rebuilds pages in the browser.

eysi09 added a commit to garden-io/garden that referenced this issue Apr 3, 2020
The latest version doesn't work on Safari and Firefox. See here:
sindresorhus/normalize-url#105
eysi09 added a commit to garden-io/garden that referenced this issue Apr 3, 2020
The latest version doesn't work on Safari and Firefox. See here:
sindresorhus/normalize-url#105
eysi09 added a commit to garden-io/garden that referenced this issue Apr 3, 2020
The latest version doesn't work on Safari and Firefox. See here:
sindresorhus/normalize-url#105
eysi09 added a commit to garden-io/garden that referenced this issue Apr 3, 2020
The latest version doesn't work on Safari and Firefox. See here:
sindresorhus/normalize-url#105
@exentrich
Copy link

It's critical issue! Page that uses normalize-url version 5 can't be opened in iOS nor macOS!

@TrySound
Copy link
Contributor

TrySound commented Jun 5, 2020

See here https://github.com/sindresorhus/normalize-url#install

@muuvmuuv
Copy link
Author

muuvmuuv commented Jun 6, 2020

It is getting some support from browsers now. Maybe some more month and v5 will be working in all major browsers as well. I'm using it in a Ionic app. Need to try if v5 works for it.

@muuvmuuv
Copy link
Author

Alternative would be to use this package. Just as long as browsers are not fully support it.

Sandbox: https://codesandbox.io/s/normalize-url-issues-105-sex88?file=/src/index.js

@Aarbel
Copy link

Aarbel commented Mar 14, 2021

I confirm that installing v4 package solves the problem on WkWebview (iOS)

npm i normalize-url@4

@whalemare
Copy link

Not worked version 6 and version 4 on iOS react-native.

Crashed with error: TypeError: Attempted to assign to readonly property.

import normalizeUrl from 'normalize-url'
const normalized = normalizeUrl('fb.com')

@fcisio
Copy link

fcisio commented May 20, 2021

Hey, I'm coming from Gatsby, thanks a lot for this issue.
As mentioned above, it's a critical issue.

And this has been open for a while. Any Idea why there hasn't been any progress on something that seems like a simple syntax error?

Thanks

@gregdingle
Copy link

I also got burned by this issue. I don't see how it's good to release new versions that are known to be broken in Safari. There must be another regex that doesn't use negative lookbehinds. @gcox ?

@gcox
Copy link
Contributor

gcox commented Jun 3, 2021

@gregdingle Not sure if you meant to reference me, but I don't see how to avoid the negative lookbehinds. I'm certainly no regex expert so I can't say it's impossible. I don't use this in a browser either, so I'm not motivated to try to work around an annoying browser limitation.

@edenhermelin
Copy link

any updates on that? or it is not planned to implement a workaround for the regex?

@muuvmuuv
Copy link
Author

Still no native support for Safari... @edenhermelin I mentioned a workaround in an earlier comment but it seems it wont be implemented.

loynoir added a commit to loynoir/normalize-url that referenced this issue Aug 7, 2021
loynoir added a commit to loynoir/normalize-url that referenced this issue Aug 7, 2021
@loynoir
Copy link

loynoir commented Aug 7, 2021

Hi, @muuvmuuv
Can you please help me test #146

I don't have available environment.

@muuvmuuv
Copy link
Author

muuvmuuv commented Aug 8, 2021

@loynoir I have not touched my portfolio a long time, I will try to. No promise.

@loynoir
Copy link

loynoir commented Aug 8, 2021

@muuvmuuv
Sorry to bother you. The real browser test in blocking by another issue.

I tried to port test.js to use mocha+chai, to let you easy test with index.html.

But unfortunatly seems at least two bugs, make browser-normalize-url stuck in v4 and cant up to v7.

  1. this one, (should be fixed?)
  2. and Error result when url using custom protocol in browser #147

@loynoir
Copy link

loynoir commented Aug 8, 2021

Did any one test v4 in real browser? So I don't have to test again myself.

loynoir added a commit to loynoir/normalize-url that referenced this issue Aug 9, 2021
1. [test] Move `ava` to `mocha+chai` to test in real browser.
(sindresorhus#105) (sindresorhus#140) (sindresorhus#142)
2. [test] seprate non special-protocol-schemes(sindresorhus#147)
3. [doc] Update readme
@loynoir
Copy link

loynoir commented Aug 9, 2021

I think, this error raise from normalize-url@v7.0.0 should be fixed in pull/146.

If anyone see this, with browser not support js-regexp-lookbehind,
you can use your real browser to test browser-test/index.html within
https://github.com/loynoir/normalize-url

You can also build that bundle using npm run build

@loynoir
Copy link

loynoir commented Aug 9, 2021

@muuvmuuv and any who volunteer to test browser version of normalize-url@7

I publish tests in git repo, so you can visit thru rawgit

https://ghcdn.rawgit.org/loynoir/normalize-url/main/browser-test/index.html

PS: just ignore title with non-special-protocol-schemes which belongs to another issue

@tuarrep
Copy link

tuarrep commented Aug 9, 2021

Just tested on my iPhone under iOS 15 beta. Two tests are failing see below.

If I remember well I’ve encountered this issue on my MacBook with Safari, I’ll test on it too later this day

AEC883C9-D2C3-43A9-9306-5CABBF46A574

@JKrol
Copy link

JKrol commented Aug 9, 2021

@loynoir here is the result from iPad, Safari:

image

@muuvmuuv
Copy link
Author

muuvmuuv commented Aug 9, 2021

Got the same as above from Windows 10 Microsoft Edge latest release

@loynoir
Copy link

loynoir commented Aug 9, 2021

  • Invalid URL

This is because of the test regexp is hard-coded, I push a commit allow less strict checking.

But seems rawgit cache haven't refresh.

https://ghcdn.rawgit.org/loynoir/normalize-url/f9f36c8f22b9649f0eac7219c31a9da2beef7896/browser-test/index.html

  • duplicate slashes
    I pick logic from v4.5.0 as fallback, it's unexpected.
    Did any one know which minor version of v4 works best?

  • special-protocol-schemes
    Kind of weird that 3/3 of you failed on same 2 errors, but I failed in a totally different 4 caused by custom protocol 'sindre://www.sorhus.com' 😂
    Under "Mozilla Firefox 90" & "Google Chrome 92"

@JKrol
Copy link

JKrol commented Aug 9, 2021

@loynoir, so after this commit (https://ghcdn.rawgit.org/loynoir/normalize-url/f9f36c8f22b9649f0eac7219c31a9da2beef7896/browser-test/index.html), you expect to fix the invalid url test fail? It did not work for me - I still get the same two failed tests on iPad (checked on Safari and Chrome).

I have also checked the Windows Chrome and Android Chrome now, and I have a different result than on iPad - it is the same as yours.

EDIT: actually there is a small change with Invlaid URL test - previously it failed with expected [Function] and now it is expected 'Type error'

@loynoir
Copy link

loynoir commented Aug 9, 2021

Folded as outdated

duplicate slashes

  • duplicate slashes
    I pick logic from v4.5.0 as fallback, it's unexpected.
    Did any one know which minor version of v4 works best?

I can reproduce that after I run into that fallback v4 logic.
If anyone interest,

  • v4.js pass v4 test.js
  • v7 test.js add many tests
  • v4 fallback logic still pass the old v4 test.js
  • v4 fallback logic not pass v7 test.js ADDED test
  • default logic pass both v4 test.js and v7 test.js

I just so trust the v4 code, I didn't comment out the default logic to reproduce. Sorry for waste your time.
@tuarrep @JKrol @muuvmuuv

For sake of testing, I think preferJsRegexpLookbehind: false or something, has to be added.

But, as v4 fallback logic not pass v7 test.js ADDED test, I write new logic using reverse + look ahead + reverse as new fallback.

  • 2 logic: look-behind and look-ahead
  • 2 period test: v4 & v7
  • 2 browser: latest chrome and latest firefox
    8 situations remove duplicate slashes pass test on my side.

https://ghcdn.rawgit.org/loynoir/normalize-url/8bace639547eae21df941c57a9cec1ed0ce994a6/browser-test/index.html

@loynoir
Copy link

loynoir commented Aug 9, 2021

@JKrol
Sorry, I can't reproduce on firefox90 and chrome92.
Maybe you can try to find what happen.
And, that test is expected to throw an error, but seems somehow it's not try-catched.
Is it fail in invalid url loose test or invalid url strict test?
If it failed in invalid url strict, that is somehow expected, because it use a regexp to detect whether the error is url error constructing.

@loynoir
Copy link

loynoir commented Aug 10, 2021

New Browser Test Link

https://ghcdn.rawgit.org/loynoir/normalize-url/0fd8bf27b3400d4cf67bce6072c3f34efa8f4e56/browser-test/index.html

Test Status

(32/32) Test passed with

  • Node.js 14
  • Chrome headless driver
  • Firefox headless driver
  • Chrome 92
  • Firefox 90
  • Firefox 68 ESR
  • Opera59
  • Yandex 7 (2017)

@gcox
Copy link
Contributor

gcox commented Aug 10, 2021

@loynoir FYI, 1 test fails in Safari 14.1.2 (16611.3.10.1.3) on macOS 11.5

Screen Shot 2021-08-10 at 11 51 06 AM

@sindresorhus
Copy link
Owner

Fixed in https://github.com/sindresorhus/normalize-url/releases/tag/v7.0.1

@loynoir
Copy link

loynoir commented Aug 11, 2021

Hi, @gcox
Can you run this in safari,

// sindresorhus/normalize-url v7.0.1
normalizeUrl('http://');

Does its error.message match /^Invalid URL/?

message: /^Invalid URL/,

As, TypeError seems too wide to match.

@loynoir
Copy link

loynoir commented Aug 11, 2021

New Browser Test Link

https://ghcdn.rawgit.org/loynoir/normalize-url/9c1db991194b0f4f69f9fd03770c77999eb5da68/browser-test/index.html

Test Status

(31/31) Test passed with

  • Node.js 14
  • Chrome headless driver
  • Firefox headless driver
  • Chrome 92
  • Firefox 90
  • Firefox 68 ESR
  • Opera59
  • Yandex 7 (2017)

@gcox
Copy link
Contributor

gcox commented Aug 11, 2021

Can you run this in safari,

// sindresorhus/normalize-url v7.0.1
normalizeUrl('http://');

Does its error.message match /^Invalid URL/?

Tried it at a break point when loading the new test page and yes, that's the error it produces.

Screen Shot 2021-08-11 at 3 20 57 PM

@loynoir
Copy link

loynoir commented Aug 11, 2021

@gcox Can you please test this? I tried to bring it back to old browser.

I tested on a 2017 browser which seems ok, but i lack of a safari env.

https://ghcdn.rawgit.org/loynoir/normalize-url/9c1db991194b0f4f69f9fd03770c77999eb5da68/browser-test/index.html

@gcox
Copy link
Contributor

gcox commented Aug 11, 2021

@loynoir All the tests pass

@loynoir
Copy link

loynoir commented Aug 11, 2021

Thanks very much! 😘

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

No branches or pull requests