Skip to content

Fix domain resolution bug#188

Merged
jamsea merged 1 commit intomasterfrom
fix/domainResolve
Feb 14, 2017
Merged

Fix domain resolution bug#188
jamsea merged 1 commit intomasterfrom
fix/domainResolve

Conversation

@jamsea
Copy link
Copy Markdown
Contributor

@jamsea jamsea commented Feb 6, 2017

No description provided.

}

function mapDomainToAlias(host, domainAliases) {
if (!isIpAddress(host)) {
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.

Why is it necessary to test for ip?

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.

IP address is a special case for wildcat. Before we introduced code to do domain aliases we weren't able to resolve them, so the logic in the rest of the function is assuming that the host isn't an IP address. Without this IP address check lines 16-18 bail out and cause tests to fail when you have a valid IP/domainAlias match

@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 9, 2017

Codecov Report

Merging #188 into master will increase coverage by 0.29%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #188      +/-   ##
==========================================
+ Coverage   99.62%   99.92%   +0.29%     
==========================================
  Files          57       38      -19     
  Lines        1342     1253      -89     
  Branches      207      205       -2     
==========================================
- Hits         1337     1252      -85     
+ Misses          5        1       -4
Impacted Files Coverage Δ
...react-wildcat-handoff/src/utils/getDomainRoutes.js 100% <100%> (+2.1%)
example/src/main.js
example/src/routes/HelmetExample/routes.js
...e/src/components/Application/ApplicationContext.js
example/src/routes/FlexboxExample/routes.js
example/src/routes.config.js
example/src/routes/IndexExample/routes.js
example/src/routes/IndexExample/IndexExample.js
example/src/routes/ErrorExample/routes.js
example/src/ApplicationRoutes.js
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47ba82a...21e36f2. Read the comment docs.

@jamsea jamsea force-pushed the fix/domainResolve branch 5 times, most recently from 7ec57b5 to 12ff734 Compare February 14, 2017 01:06
Wildcat would resolve to the wrong domain if domain aliases had a null/undefined value
@jamsea jamsea merged commit c331a13 into master Feb 14, 2017
@doctyper doctyper mentioned this pull request Apr 10, 2017
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.

3 participants