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

stripAuthentication does not work as documented #184

Closed
rendall opened this issue Jul 16, 2023 · 3 comments
Closed

stripAuthentication does not work as documented #184

rendall opened this issue Jul 16, 2023 · 3 comments

Comments

@rendall
Copy link
Contributor

rendall commented Jul 16, 2023

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
Copy link
Contributor Author

rendall commented Jul 16, 2023

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
Copy link
Owner

sindresorhus commented Jul 16, 2023

This is by design. I just forgot to document the behavior: 2d0bafe

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

@sindresorhus sindresorhus closed this as not planned Won't fix, can't repro, duplicate, stale Jul 16, 2023
@rendall
Copy link
Contributor Author

rendall commented Jul 16, 2023

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

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

No branches or pull requests

2 participants