-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix: Handle case-insensitive www. prefix
#3769
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @vaskkey, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where the autolinking mechanism failed to correctly prepend the "http://" protocol to URLs that began with a case-insensitive "www." prefix. By updating the tokenizer's logic to perform a case-insensitive check, the system now ensures that all variations of "www." (e.g., "Www.", "WWW.") are properly recognized and converted into fully functional, clickable links, preventing broken redirections. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request fixes an issue where www. prefixes in URLs were not handled case-insensitively for autolinking. The change correctly identifies prefixes like Www. and prepends http://. The implementation is correct according to the goal, and new tests are added to cover the case-insensitive scenarios. I have one suggestion for improvement regarding URL normalization for better standards compliance.
| } while (prevCapZero !== cap[0]); | ||
| text = cap[0]; | ||
| if (cap[1] === 'www.') { | ||
| if (cap[1]?.toLowerCase() === 'www.') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change correctly handles case-insensitive www. prefixes. However, the generated href preserves the original casing of the domain (e.g., http://Www.foo.com).
According to RFC 3986, the host component of a URI is case-insensitive. It is a best practice to normalize it to lowercase in the href attribute for consistency and to prevent potential issues. The visible link text should preserve the original user input.
You could consider updating the logic inside this if block to normalize the domain part of the URL. Here's a possible implementation:
// inside the if (cap[1]?.toLowerCase() === 'www.') block
const pathStartIndex = cap[0].search(/[\/?#]/);
const domain = pathStartIndex === -1 ? cap[0] : cap[0].substring(0, pathStartIndex);
const path = pathStartIndex === -1 ? '' : cap[0].substring(pathStartIndex);
href = 'http://' + domain.toLowerCase() + path;If you apply this change, please remember to update the corresponding test case in test/specs/new/autolinks.html to expect a lowercase href.
|
Autolink is a GFM extension so we want it to act the same as GitHub and it looks like GitHub doesn't allow the www to be case insensitive. www.example.com If this is something you want you can create a custom extension |
|
@UziTech In that case I could probably configure the autolink to be a bit stricter than it is right now. because currently for Www.example.com |
|
Good call. Ya that sounds good. We want it to be the same as GitHub |
|
Closing this PR, Github-like implementation here: #3770 |
Marked version:
16.3.0
Markdown flavor: Markdown.pl|CommonMark|GitHub Flavored Markdown|n/a
Description
Expectation
When a link starts with
www.(in any case combination likeWww.,WWW., etc.), it should be automatically converted to a clickable link withhttp://prepended.Result
Before the fix, only lowercase
www.was being prepended withhttp://. Links starting withWww.or other case variations were being processed as autolinks but did not havehttp://prepended, which caused the links to redirect tohttp://mainwebsite/Www.foo.com.After the fix, both
www.foo.comandWww.foo.comare now properly converted to<a href="http://www.foo.com">www.foo.com</a>and<a href="http://Www.foo.com">Www.foo.com</a>respectively.What was attempted
The fix was implemented in
src/Tokenizer.ts:843by changing the strict equality check from:to a case-insensitive comparison:
Contributor
Committer
In most cases, this should be a different person than the contributor.