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

Require a proper URL with HTTPS only when adding link #2328

Merged
merged 6 commits into from
Feb 5, 2025

Conversation

MaPoKen
Copy link
Contributor

@MaPoKen MaPoKen commented Jan 31, 2025

@MaPoKen MaPoKen requested a review from a team January 31, 2025 09:10
Copy link
Contributor

@katrinewi katrinewi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tror ikke valideringen er helt riktig, jeg får feks lov til å skrive url på formatet: hhttp://nrk.no

src/messages/messagesEN.ts Outdated Show resolved Hide resolved
src/messages/messagesEN.ts Outdated Show resolved Hide resolved
@MaPoKen MaPoKen force-pushed the learningpath-require-proper-url branch from e363778 to 8541e33 Compare February 4, 2025 08:33
Comment on lines 47 to 54
const isValidURL = (url: string) => {
try {
new URL(url);
return true;
} catch {
return false;
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dette holder vel ikke for riktig validering? 🤔
Må det ikke være noe sånt som:

const isValidURL = (url: string) => {
  try {
    const parsedUrl = new URL(url);
    return ["http:", "https:"].includes(parsedUrl.protocol);
  } catch {
    return false;
  }
};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hvorfor bruker vi ikke regex, egentlig? Har vi ikke en URL-regex som dekker behovene våre i AutoLinkPlugin? Kan jo bare dra den ut

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forslaget mitt var fordi jeg føler det er lettere å innføre feil validering med regex, men man får jo også muligheten til å tune akkurat hvordan valideringen skal være, så kanskje greit om vi bruker det andre steder!

@MaPoKen MaPoKen requested review from katrinewi and Jonas-C February 4, 2025 11:21
@katrinewi
Copy link
Contributor

katrinewi commented Feb 4, 2025

Valideringen plukker fortsatt ikke opp: hhttps://nrk.no, er det meningen?

@MaPoKen MaPoKen force-pushed the learningpath-require-proper-url branch from 288009d to 34a0274 Compare February 4, 2025 15:09
@gunnarvelle gunnarvelle merged commit 90c79a4 into master Feb 5, 2025
6 checks passed
@gunnarvelle gunnarvelle deleted the learningpath-require-proper-url branch February 5, 2025 13:16
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

Successfully merging this pull request may close these issues.

4 participants