Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 19 additions & 16 deletions src/utils/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
* limitations under the License.
*/

import url = require('url');

/**
* Validates that a value is a byte buffer.
*
Expand Down Expand Up @@ -234,33 +232,38 @@ export function isURL(urlStr: any): boolean {
return false;
}
try {
const uri = url.parse(urlStr);
const uri = new URL(urlStr);
const scheme = uri.protocol;
const slashes = uri.slashes;
const hostname = uri.hostname;
const pathname = uri.pathname;
if ((scheme !== 'http:' && scheme !== 'https:') || !slashes) {
if (scheme !== 'http:' && scheme !== 'https:') {
return false;
}
// Validate hostname: Can contain letters, numbers, underscore and dashes separated by a dot.
// Each zone must not start with a hyphen or underscore.
if (!hostname || !/^[a-zA-Z0-9]+[\w-]*([.]?[a-zA-Z0-9]+[\w-]*)*$/.test(hostname)) {
return false;
const hostname = uri.hostname;
// Validate hostname strictly to match previous behavior and prevent weak/invalid domains.
// Must be alphanumeric with optional dashes/underscores, separated by dots.
// Cannot start/end with dot or dash (mostly).
// This regex is safe (no nested quantifiers with overlap).
if (!/^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)*$/.test(hostname)) {
// Check for IPv6 literals which are valid but behave differently.
// Node 'new URL' keeps brackets for IPv6: [::1] -> [::1]
// Check for IPv6 address (simple check for brackets)
if (!/^\[[a-fA-F0-9:.]+\]$/.test(hostname)) {
return false;
}
}
Comment on lines +245 to 252

Choose a reason for hiding this comment

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

medium

The nested if condition for hostname validation is a bit difficult to read. You can improve clarity by flattening the logic. Check for a valid domain and a valid IPv6 literal separately and then combine the results. This makes the code's intent—that the hostname must be one of the two—more explicit.

    const isValidDomain = /^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)*$/.test(hostname);
    // An IPv6 literal is enclosed in brackets. `new URL()` has already validated the contents.
    const isIPv6Literal = /^\[[a-fA-F0-9:.]+\]$/.test(hostname);

    if (!isValidDomain && !isIPv6Literal) {
      return false;
    }

// Allow for pathnames: (/chars+)*/?
// Restore strict pathname validation: (/chars+)*/?
// Where chars can be a combination of: a-z A-Z 0-9 - _ . ~ ! $ & ' ( ) * + , ; = : @ %
const pathnameRe = /^(\/[\w\-.~!$'()*+,;=:@%]+)*\/?$/;
// Validate pathname.
const pathname = uri.pathname;
if (pathname &&
pathname !== '/' &&
!pathnameRe.test(pathname)) {
pathname !== '/' &&
!pathnameRe.test(pathname)) {
return false;
}
// Allow any query string and hash as long as no invalid character is used.
return true;
} catch (e) {
return false;
}
return true;
}


Expand Down
25 changes: 25 additions & 0 deletions test/unit/utils/validator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -530,3 +530,28 @@ describe('isISODateString()', () => {
expect(isISODateString(validISODateString)).to.be.true;
});
});

describe('isURL() ReDoS and Long Inputs', () => {
it('should handle long valid URLs quickly', function () {
this.timeout(1000);
const longUrl = 'https://' + Array(50).fill('a').join('.') + '.com';
expect(isURL(longUrl)).to.be.true;
});

it('should handle long invalid URLs quickly (ReDoS check)', function () {
this.timeout(1000);
const longInvalid = 'https://' + 'a'.repeat(22) + '!';
expect(isURL(longInvalid)).to.be.false;
});

it('should handle very long domain with many segments', function () {
this.timeout(1000);
const manySegments = 'https://' + Array(100).fill('a').join('.') + '.com';
expect(isURL(manySegments)).to.be.true;
});

it('should reject invalid dot usage caught by strict regex', function () {
expect(isURL('https://a.b')).to.be.true;
expect(isURL('https://a..b')).to.be.false;
});
});
Loading