Application URLs with international characters are considered invalid

[augustuen]

Description

Trying to set an application URL with international chararacters (ex: Æ Ø Å) doesn't pass validation.

Version

2.5.1

Steps to Reproduce

1. Go to Settings->General
2. Enter a URL with a special characters (for example Æ, Ø, or Å) into the "Application URL" field.
3. Observe the "You must provide a valid URL" validation error appear below the field.

Screenshots

Logs

Platform

desktop

Database

SQLite (default)

Device

Window Desktop

Operating System

Windows 10

Browser

Firefox 136.0.2

Additional Context

No response

Code of Conduct

• I agree to follow Jellyseerr's Code of Conduct

[augustuen]

Looks like this section is the cause:

jellyseerr/src/components/Settings/SettingsMain/index.tsx

Lines 73 to 77 in 29034b3

	applicationUrl: Yup.string()
	.matches(
	/^https?:\/\/(www\.)?[-a-zA-Z0-9@:%._+~#=]{1,256}(\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_+.~#?&/=]*))?$/i,
	intl.formatMessage(messages.validationApplicationUrl)
	)

As far as I can tell, the RegEx doesn't include these kinds of characters. I'd submit a PR for it, if RegEx wasn't greek to me.

[gauthier-th]

Looks like this section is the cause:

jellyseerr/src/components/Settings/SettingsMain/index.tsx

Lines 73 to 77 in 29034b3

	applicationUrl: Yup.string()
	.matches(
	/^https?:\/\/(www\.)?[-a-zA-Z0-9@:%._+~#=]{1,256}(\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_+.~#?&/=]*))?$/i,
	intl.formatMessage(messages.validationApplicationUrl)
	)

As far as I can tell, the RegEx doesn't include these kinds of characters. I'd submit a PR for it, if RegEx wasn't greek to me.

We should even use something else instead of these regexs.
This is not really a good pratice since these hand-crafted regex will likely fail at some point (like right now)

[ankarhem]

Relevant changes occured in:

Feature added: 026795d
Validation changed: 14f316a

Initially I wondered why this didnt just use the built in url validation from yup, but given the commit message in 14f316a it seems that this is less strict than that.

Since we have some duplication for this validation as seen in 14f316a, I suggest we create a module for validating url (try-catch on new URL) and mapping to / from punycode. Might want some additional logic to still show non-punycode value in the fields.

    applicationUrl: Yup.string()
      .test(
        'is-url',
        intl.formatMessage(messages.validationApplicationUrl),
        (value) => {
          if (!value) return true; // Allow empty value
          return isValidURL(toPunyCode(value))
        }
      )
      .test(
        'no-trailing-slash',
        intl.formatMessage(messages.validationApplicationUrlTrailingSlash),
        (value) => !value || !value.endsWith('/')
      ),

[gauthier-th]

Since we have some duplication for this validation as seen in 14f316a, I suggest we create a module for validating url (try-catch on new URL) and mapping to / from punycode.

Yes, I'm in favor of using the URL constructor. It could be as simple as:

function isValidURL(text: string) {
  try {
    const url = new URL(text);
    return url.protocol === 'http:' || url.protocol === 'https:';
  catch {
    return false;
  }
}

Might want some additional logic to still show non-punycode value in the fields.

I don't understand what you mean with that.

[ankarhem]

Might want some additional logic to still show non-punycode value in the fields.

I don't understand what you mean with that.

If we save the non-punycode (IDN-variant) value of the url I am worried some links might not work, depending if they are just interpolated or constructed. I'm unsure if that is really the case, but the implementer probably want to make sure that works. My worry is that use-cases like this: ${applicationUrl}/some/link could fail, while use cases using the URL-constructer would work.

If the worry is a real concern, my suggestion was to transform the input-value, and save the punycode. We could use the URL-constructor here as well.

However, this would cause the input fields to change upon refetch after save. We could prevent this by mapping the value to / from punycode so that the user still sees what they entered (the IDN-version).

[gauthier-th]

some links might not work

AFAIK it shouldn't be the case. We use axios everywhere in the app, and axios should support it, so i don't really see the point in managing encoding/decoding manually.
If we come across a case where this is a problem, or if you have an example where it wouldn't work in your head, then no problem with encoding/decoding.