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

fixes Comcast/parodus#194 parse_webpa_url function doesn't correctly return server address #195

Merged
merged 1 commit into from
May 15, 2018

Conversation

nunogmartins
Copy link
Contributor

If the redirect from Petasos returns a URL which has an URI with "/api/v2/" the
parse_webpa_url function must only return in address_buffer the server
and not the server/api/v2/ as shown in the test case

… server address if it doesn't have port number

If the redirect returns a URL which has an URI with "/api/v2/" the
parse_webpa_url function must only return in address_buffer the server
and not the server/api/v2/ as shown in the test case
@CLAassistant
Copy link

CLAassistant commented May 15, 2018

CLA assistant check
All committers have signed the CLA.

@codecov-io
Copy link

codecov-io commented May 15, 2018

Codecov Report

Merging #195 into master will decrease coverage by <.01%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #195      +/-   ##
==========================================
- Coverage   88.04%   88.03%   -0.01%     
==========================================
  Files          41       41              
  Lines        4425     4431       +6     
  Branches      358      359       +1     
==========================================
+ Hits         3896     3901       +5     
  Misses        198      198              
- Partials      331      332       +1
Impacted Files Coverage Δ
tests/test_config.c 96.96% <100%> (+0.03%) ⬆️
src/config.c 74.26% <66.66%> (-0.07%) ⬇️

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 1da5978...7b1ef04. Read the comment docs.

@nunogmartins
Copy link
Contributor Author

nunogmartins commented May 15, 2018

I'm seeing that there is a red line for src/config.c file from code coverage, however it is not from the code I changed. I'm not sure what is your code style for curly brackets on the if statement.

@bill1600
Copy link
Collaborator

codecov is crazy.

@@ -463,6 +463,10 @@ void test_parse_webpa_url ()
addr_buf, 80, port_buf, 8), 0);
assert_string_equal (addr_buf, "mydns.mycom.net");
assert_string_equal (port_buf, "443");
assert_int_equal (parse_webpa_url ("https://mydns.mycom.net/api/v2/",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't even know this was an acceptable url.
But if so, we have to handle it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At our company we are seeing it. We are seeing a redirect that has an URI in it, and as we are using default HTTPS port, this bug was easily spotted.

I don't know if you @bill1600 or @schmidtw could merge it, if it also fits Comcast needs.
If it a reshape in any way, I'm all eyes and ears for your comments.

@schmidtw schmidtw merged commit 0618690 into xmidt-org:master May 15, 2018
@schmidtw
Copy link
Member

Good catch - thanks @nunogmartins !

@nunogmartins
Copy link
Contributor Author

Thanks @schmidtw

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.

5 participants