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

Fix check segment_result range(for node issue #51514) #581

Merged
merged 2 commits into from
Jan 21, 2024

Conversation

ChenKS12138
Copy link
Contributor

@ChenKS12138 ChenKS12138 commented Jan 21, 2024

Fix an intermittent issue identified by @danfuzz in nodejs/node#51514

Reproudction

Code Example

#include "ada.cpp"
#include "ada.h"
#include <iostream>

int main(int, char *[]) {
  auto url = ada::parse<ada::url>("https://1.1.1.256");
  if (!url) {
    std::cout << "failure" << std::endl;
    return EXIT_FAILURE;
  }
  url->set_protocol("http");
  std::cout << url->get_protocol() << std::endl;
  std::cout << url->get_host() << std::endl;
  return EXIT_SUCCESS;
}

Expected

Raise an error and stop parse.

image

Actual

Successfully parsed.

image

Reason

This is an edge case, segment_result should not be equal to the size of remaining bits.

ada/src/url.cpp

Lines 64 to 83 in 4814d2f

if (input.empty()) {
// We have the last value.
// At this stage, ipv4 contains digit_count*8 bits.
// So we have 32-digit_count*8 bits left.
if (segment_result > (uint64_t(1) << (32 - digit_count * 8))) {
return is_valid = false;
}
ipv4 <<= (32 - digit_count * 8);
ipv4 |= segment_result;
goto final;
} else {
// There is more, so that the value must no be larger than 255
// and we must have a '.'.
if ((segment_result > 255) || (input[0] != '.')) {
return is_valid = false;
}
ipv4 <<= 8;
ipv4 |= segment_result;
input.remove_prefix(1); // remove '.'
}

tests/basic_tests.cpp Outdated Show resolved Hide resolved
@anonrig anonrig merged commit 219ac56 into ada-url:main Jan 21, 2024
35 checks passed
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