sindresorhus / normalize-url

Normalize a URL
MIT License
837 stars 123 forks source link

stripAuthentication does not work as documented #184

Closed rendall closed 1 year ago

rendall commented 1 year ago

The documentation has these examples regarding stripAuthentication:

normalizeUrl('user:password@sindresorhus.com'); //=> 'https://sindresorhus.com'

normalizeUrl('user:password@sindresorhus.com', {stripAuthentication: false}); //=> 'https://user:password@sindresorhus.com'

However, those examples fail in (Jest) tests:

normalizeUrl › should strip authentication part of the URL by default

    Expected: "https://sindresorhus.com"
    Received: "user:password@sindresorhus.com"

      24 |   it('should strip authentication part of the URL by default', () => {
    > 25 |     expect(normalizeUrl('user:password@sindresorhus.com')).toBe('https://sindresorhus.com');
         |                                                            ^
      26 |   });

      at Object.<anonymous> (src/tests/backend/normalizeUrl.test.ts:25:60)

  ● normalizeUrl › should strip authentication part of the URL if stripAuthentication is true

    Expected: "https://sindresorhus.com"
    Received: "user:password@sindresorhus.com"

      28 |   it('should strip authentication part of the URL if stripAuthentication is true', () => {
    > 29 |     expect(normalizeUrl('user:password@sindresorhus.com', { stripAuthentication: true })).toBe('https://sindresorhus.com');
         |                                                                                           ^
      30 |   });  

      at Object.<anonymous> (src/tests/backend/normalizeUrl.test.ts:29:91)

These tests do pass, however, if the protocol is included:

  it('should strip authentication part of the URL by default', () => {
    expect(normalizeUrl('https://user:password@sindresorhus.com')).toBe('https://sindresorhus.com');
  });

  it('should strip authentication part of the URL if stripAuthentication is true', () => {
    expect(normalizeUrl('https://user:password@sindresorhus.com', { stripAuthentication: true })).toBe('https://sindresorhus.com');
  });  

  it('should not strip authentication if stripAuthentication is false', () => {
    expect(normalizeUrl('https://user:password@sindresorhus.com', { stripAuthentication: false })).toBe('https://user:password@sindresorhus.com');
  });
rendall commented 1 year ago

This test passes, asking it to strip authentication and protocol:

  it('should strip authentication and protocol', () => {
    expect(normalizeUrl('https://user:password@sindresorhus.com', {stripProtocol:true}))
        .toBe('sindresorhus.com');
  });
sindresorhus commented 1 year ago

This is by design. I just forgot to document the behavior: https://github.com/sindresorhus/normalize-url/commit/2d0bafe20423c9424266b63e0b5086fc38364127

Also see: https://github.com/sindresorhus/normalize-url/releases/tag/v8.0.0

rendall commented 1 year ago

Understood. My comment was more about the documentation. I submitted PR #185 as a quick documentation fix to get it aligned with the behavior.