JavaScript error in vault name regex upon vault creation

[chenkins]

Please agree to the following

• [X] I have searched existing issues for duplicates
• [X] I agree to follow this project's Code of Conduct

Summary

JavaScript error in vault name regex upon vault creation

System Setup

- Hub: 1.4.0-SNAPSHOT
- Keycloak: 23.0.7
- Cryptomator --
- …

Steps to Reproduce

1. create vault /app/vaults/create, start typing vault name

Expected Behavior

No JavaScript error see log output.

Actual Behavior

JavaScript error see log output.

Reproducibility

Always

Relevant Log Output

Unable to check <input pattern='^(?! )([^\\\/:*?\x22<>|])*(?<! |\.)$'> because the pattern is not a valid regexp: invalid character in class in regular expression

Anything else?

Regex is pretty old (https://github.com/cryptomator/hub/commit/e840c222103c5db59161c38e393af6cb51bb9aea #129), I see it in the JavaScript console in Firefox, but not in Safari. Is the problem known?

[tobihagemann]

Didn't know that this happens in Firefox as well, but I noticed this issue at some point after a Chrome update. It was "just" a warning in the past, but now it's an error. I guess, we have to fix it, because the vault name validation doesn't work anymore. I don't know what the problem is, but my guess is that we have to escape some special characters now.

[chenkins]

The regex seems to work fine here: https://regex101.com/

The problem seems to be in the negative lookbehind part.

NOK:

it('regex()', async () => {
     const regex = new RegExp("^(?! )([^\\\/:*?\x22<>|])*(?<! |\.)$");
     expect(regex.test("TRESOR")).to.eq(true);
})

OK:

it('regex()', async () => {
     const regex = new RegExp("^(?! )([^\\\/:*?\x22<>|])*$");
     expect(regex.test("TRESOR")).to.eq(true);
})

[tobihagemann]

Did some experiments, and I still think it's just a missing escape.

^(?! )([^\\\/:*?\x22<>\|])*(?<! |\.)$

Seems to work in Chrome, and I just added a \ in front of the first |.

Edit: I don't know about the negative lookbehind, but it doesn't seem to bother Chrome. Basically, we're trying to find a rule for the following:

Disallowed characters are: \ / : * ? " < > | Furthermore, the vault name cannot end with a period or space.

[chenkins]

Manually tested in Firefox ✅

Using hex notation:

^(?! )([^\x5C\x5C\x2F:*?\x22<>\x7C])*(?<![ \x7C\x2E])$

it works both in the browser (manually tested in FF) as well as in a unit test (as documentation):

it('regex()', async () => {
     const regex = new RegExp("^(?! )([^\x5C\x5C\x2F:*?\x22<>\x7C])*(?<![ \x7C\x2E])$");
     // allowed characters
     // TODO should be plus instead of * in the regex?
     expect(regex.test("")).to.eq(true);
     expect(regex.test("TRESOR")).to.eq(true);
     expect(regex.test("TR!ESOR")).to.eq(true);
     expect(regex.test("TR_ESOR")).to.eq(true);
     expect(regex.test("tresor")).to.eq(true);
     expect(regex.test("1234")).to.eq(true);
     // disallowed characters \ / : * ? " < > |
     expect(regex.test("TR\\SOR")).to.eq(false);
     expect(regex.test("TR/SOR")).to.eq(false);
     expect(regex.test("TR:SOR")).to.eq(false);
     expect(regex.test("TR*SOR")).to.eq(false);
     expect(regex.test("TR?SOR")).to.eq(false);
     expect(regex.test("TR\"SOR")).to.eq(false);
     expect(regex.test("TR<SOR")).to.eq(false);
     expect(regex.test("TR>SOR")).to.eq(false);
     expect(regex.test("TR|SOR")).to.eq(false);
     // initial negative lookahead: <space>
     expect(regex.test(" TRESOR")).to.eq(false);
     // final negative lookbehind: | . <space>
     expect(regex.test("TRESOR|")).to.eq(false);
     expect(regex.test("TRESOR ")).to.eq(false);
     expect(regex.test("TRESOR.")).to.eq(false);
})

[tobihagemann]

I've noticed two things:

1. \x5C is twice in the expression. I believe once is enough?
2. I think you can delete the last \x7C. Originally, it was meant to be an OR. But you rewrote the expression using []. The final negative look behind should only look for period and space. | is already in the list of disallowed characters.

[chenkins]

1. With just one \x5C, it did not work in the new RegExp("...") idiom in spec.ts but in browser... Following https://stackoverflow.com/questions/16648679/regexp-in-typescript, works with only one \x5C using /.../ idiom instead.
2. Agree.

Working in spec and browser:

it('regex()', async () => {
     const regex = /^(?! )([^\x5C\x2F:*?\x22<>\x7C])+(?<![ \x2E])$/;
     // non-empty
     expect(regex.test("")).to.eq(false);
     // allowed characters
     expect(regex.test("TRESOR")).to.eq(true);
     expect(regex.test("TR!ESOR")).to.eq(true);
     expect(regex.test("TR_ESOR")).to.eq(true);
     expect(regex.test("tresor")).to.eq(true);
     expect(regex.test("1234")).to.eq(true);
     // disallowed characters \ / : * ? " < > |
     expect(regex.test("TR\\SOR")).to.eq(false);
     expect(regex.test("TR/SOR")).to.eq(false);
     expect(regex.test("TR:SOR")).to.eq(false);
     expect(regex.test("TR*SOR")).to.eq(false);
     expect(regex.test("TR?SOR")).to.eq(false);
     expect(regex.test("TR\"SOR")).to.eq(false);
     expect(regex.test("TR<SOR")).to.eq(false);
     expect(regex.test("TR>SOR")).to.eq(false);
     expect(regex.test("TR|SOR")).to.eq(false);
     expect(regex.test("TRESOR|")).to.eq(false);
     expect(regex.test("|TRESOR")).to.eq(false);
     // initial negative lookahead: <space>
     expect(regex.test(" TRESOR")).to.eq(false);
     // final negative lookbehind: . <space>
     expect(regex.test("TRESOR ")).to.eq(false);
     expect(regex.test("TRESOR.")).to.eq(false);
})

[infeo]

I also tested the expression ^(?! )([^\x5C\x2F:*?\x22<>\x7C])+(?<![ \x2E])$ with different browsers and via npm run test:

• npm: ✅
• Firefox: ✅
• Chrome: ✅
• Edge (which is kinda chrome): ✅

@tobihagemann Can you test Safari?

If the experiments yield a positive result everywhere, i'll replace the old regex.

[tobihagemann]

Safari works fine as well.