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

[Fix] URL inputs to not submit numeric hostnames and url without domain #6482

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

Nabhag8848
Copy link
Contributor

@Nabhag8848 Nabhag8848 commented Jul 31, 2024

ISSUE (BUG)

Description

  • When given a string like https://300, it interprets 300 as the hostname, which it then converts to an IP address, what i did was to check if the entire string (ignoring the protocol ) is number then don't submit it all within zod and it makes sense to do that cause.

  • The range for valid 32-bit unsigned integers is 0 to 4294967295 (which corresponds to 0.0.0.0 to 255.255.255.255) so other than this numbers by default URL objects throws invalid.

Screen.Recording.2024-08-01.at.1.25.30.AM.mov

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

The changes enhance URL validation to prevent numeric hostnames and URLs without a top-level domain (TLD) from being considered valid.

  • Modified packages/twenty-front/src/utils/validation-schemas/absoluteUrlSchema.ts to include checks for numeric-only strings and ensure the presence of a TLD.
  • Updated absoluteUrlSchema to transform input strings and validate them against new criteria, addressing the issue of numeric strings being interpreted as IP addresses.

1 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings

@Nabhag8848
Copy link
Contributor Author

Nabhag8848 commented Jul 31, 2024

Hey twenty team, I'm Nabhag, not sure whom to Tag, @Weiko @Bonapara @FelixMalfait I have few more questions before we can review this:

  • If the url is not valid and user Press the Enter should user go out of focus from that cell or it just shouldn't submit as seen on Demo Video above ?
  • as suggested by greptile for tlds do we need strict checking for tlds if so, we can use something like -> https://www.npmjs.com/package/tlds or lemme know if there is something already for this.

@Nabhag8848
Copy link
Contributor Author

Nabhag8848 commented Jul 31, 2024

also I see, should links support something like localhost:3000 <- is this the edge case or overkill ?

is there any other case y'all have in mind?

Copy link
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

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

LGTM thanks @Nabhag8848

@Weiko Weiko merged commit 320742c into twentyhq:main Aug 8, 2024
6 of 11 checks passed
Copy link

github-actions bot commented Aug 8, 2024

Thanks @Nabhag8848 for your contribution!
This marks your 0th PR on the repo. You're top 0% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

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

Successfully merging this pull request may close these issues.

URL Hostname from link fields shows IP address when providing ip format
3 participants