Skip to content

[INS-1054] Remove .git suffix requirement for custom git repository URL - #9086

Merged
godfrzero merged 4 commits into
developfrom
fix/INS-1054-git-url
Sep 16, 2025
Merged

[INS-1054] Remove .git suffix requirement for custom git repository URL#9086
godfrzero merged 4 commits into
developfrom
fix/INS-1054-git-url

Conversation

@godfrzero

Copy link
Copy Markdown
Contributor

Not all providers will use a URL ending in .git. The requirement makes sense for Github/Gitlab since we know which conventions they follow, but not for a form which can connect to any git server. For example, Azure DevOps repo URLs don't end in .git and customers report that manually adding it results in a 404.

Note that I have not been able to test cloning an Azure DevOps repo myself, so this PR just makes two changes:

  • Modifies validation to no longer require the URL to end in .git when adding a URL in the Git—a.k.a. custom repository settings—form.
  • Requires at least one character to be present after the URL scheme for the validation to pass.

@godfrzero
godfrzero requested a review from pavkout September 3, 2025 05:56
@pavkout
pavkout force-pushed the fix/INS-1054-git-url branch from 36aae8a to c572aa2 Compare September 4, 2025 10:37
<Input
type="url"
pattern="https?://.*\.git"
pattern="https?://.+"

@pavkout pavkout Sep 4, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@godfrzero After testing this PR I noticed the following issue

The pattern we use now https?://.+ it will match spaces, symbols, or query strings that might not be valid URLs.

  • Examples 1
  1. https://github.com/foo bar
  2. https://github.com/@@@!!
image image
  • Example 2

Allow multiple URLs
Screenshot 2025-09-04 at 13 50 10

Screenshot 2025-09-04 at 13 51 53

It's better to use this pattern https?://[^\s]+. This pattern is a safer choice for matching most common URLs with a path, while avoiding false matches or truncated links.

  • Requires domain
  • Stops at spaces
  • Still allows .git or no .git

@pavkout

pavkout commented Sep 4, 2025

Copy link
Copy Markdown
Contributor

@godfrzero An alternative approach would be to rely on the browser’s built-in validation instead of enforcing a custom pattern, since the .git suffix is optional. I’ve also applied a small improvement to the parseGitToHttpsURL function to prevent potential crashes. This keeps the implementation simpler and more robust overall.

@pavkout
pavkout force-pushed the fix/INS-1054-git-url branch from 4739972 to 0e16a29 Compare September 4, 2025 14:36
@godfrzero

Copy link
Copy Markdown
Contributor Author

Refraining from merging this for now because I'm seeing some strange behavior in the local build. Will test more tomorrow and figure out what's going on.

@gatzjames
gatzjames force-pushed the fix/INS-1054-git-url branch from 77bdd30 to c47a31f Compare September 5, 2025 08:16
@godfrzero

Copy link
Copy Markdown
Contributor Author

Refraining from merging this for now because I'm seeing some strange behavior in the local build. Will test more tomorrow and figure out what's going on.

This was happening because the .git requirement was re-added when #9092 was merged. I've removed the requirement again and the GUI now accepts an Azure DevOps URL, e.g. https://dev.azure.com/fabrikam/Fabrikam-Fiber-Git/_git/Fabrikam

// final URL fallback for any other git URL
temp = new URL(temp).href;
parsed = temp;
tempURL = (URL.canParse(tempURL) ? URL.parse(tempURL)?.href : url) || '';

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@pavkout Should the code not throw an error instead of settling on '' in case everything fails?

@pavkout pavkout Sep 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@godfrzero This function’s purpose is to parse the given URL. In Node.js, calling new URL(temp).href behaves differently than in the browser, which can lead to issues. To handle that discrepancy, we introduced this logic. The actual validation, is performed on the client side.

@pavkout
pavkout force-pushed the fix/INS-1054-git-url branch from a6559fc to e209f55 Compare September 16, 2025 11:57

@pavkout pavkout left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@godfrzero

Copy link
Copy Markdown
Contributor Author

Verified with an Azure DevOps repo that cloning, pushing, and pulling changes all work as expected.

@godfrzero
godfrzero merged commit 8c43ed9 into develop Sep 16, 2025
14 checks passed
@godfrzero
godfrzero deleted the fix/INS-1054-git-url branch September 16, 2025 21:58
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.

2 participants