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

feat: Allow wildcard hostnames #347

Merged
merged 1 commit into from
Sep 18, 2019
Merged

Conversation

wwilsman
Copy link
Contributor

@wwilsman wwilsman commented Sep 18, 2019

Purpose

When the subdomain of a hostname is variable, not known, or has many other subdomains, it is more concise to use a wildcard subdomain.

Approach

Attempted to use domainMatch to compare the resource hostname with the allowed hostnames. However internally, the package uses parsedURL.host which contains the port number and causes the check to fail. The source code was fairly small and straightforward, so I copied it here and adjusted it to use hostname which does not contain the port number.

Copying the small util here also made typescript happy about missing type defs.

@wwilsman wwilsman requested a review from Robdel12 September 18, 2019 16:31
Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

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

🏁 Looks good to me 😃

@wwilsman
Copy link
Contributor Author

RIP. The asset wasn't captured. Let me see why locally

package.json Show resolved Hide resolved
@wwilsman wwilsman force-pushed the ww/allow-wildcard-hostnames branch from 2a99a7f to bf25dc3 Compare September 18, 2019 20:01
@wwilsman
Copy link
Contributor Author

wwilsman commented Sep 18, 2019

Unrelated, it looks like the CSSOM test flaked in one of the CI runs 🤔

https://circleci.com/gh/percy/percy-agent/3019

@Robdel12
Copy link
Contributor

Robdel12 commented Sep 18, 2019

Oh weird, that looks like the live site failed to be requested or something 🤔 Another case for #304

@wwilsman wwilsman force-pushed the ww/allow-wildcard-hostnames branch from bf25dc3 to 06b3a28 Compare September 18, 2019 20:28
@wwilsman
Copy link
Contributor Author

The domain-match package had an issue where it was comparing hosts including the port number. Updated the approach (and PR description) to copy the small util over and update it to not use the port number.

@Robdel12 could you re-review, including that new util file?

@wwilsman wwilsman requested a review from Robdel12 September 18, 2019 20:33
Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

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

🏁 Very nice, I like the new approach a lot more. Less dependencies

@wwilsman wwilsman merged commit cc6ef9d into master Sep 18, 2019
@wwilsman wwilsman deleted the ww/allow-wildcard-hostnames branch September 18, 2019 20:35
wwilsman added a commit that referenced this pull request Sep 18, 2019
djones pushed a commit that referenced this pull request Sep 18, 2019
# [0.14.0](v0.13.0...v0.14.0) (2019-09-18)

### Features

* ✨ Allow wildcard hostnames ([#347](#347)) ([730161f](730161f))
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