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

Hex validator returns invalid hex string #1790

Open
2 of 4 tasks
martinboksa opened this issue Jan 26, 2023 · 4 comments
Open
2 of 4 tasks

Hex validator returns invalid hex string #1790

martinboksa opened this issue Jan 26, 2023 · 4 comments
Labels
stage/1-reproduction A reproduction exists

Comments

@martinboksa
Copy link

martinboksa commented Jan 26, 2023

Issue workflow progress

Progress of the issue based on the Contributor Workflow

  • 1. The issue provides a reproduction available on GitHub, Stackblitz or CodeSandbox

Make sure to fork this template and run yarn generate in the terminal.

Please make sure the GraphQL Scalars package version under package.json matches yours.

  • 2. A failing test has been provided
  • 3. A local solution has been provided
  • 4. A pull request is pending review

Describe the bug
I tried to send 'hex' data to graphql server and store them in database (as BYTEA column in postgre) but hexValidator always returns an error.

IDK why is not working properly, maybe the fix could be to update hexValidator() function into regexp.

To Reproduce
https://codesandbox.io/p/sandbox/sweet-gianmarco-1bldd6?file=%2Fsrc%2Findex.ts

    const binaryData = new Uint8Array([
      1, 3, 255, 184, 227, 157, 8, 0, 7, 1, 7, 100, 101, 102, 97, 117, 108, 116,
      3, 13, 100, 111, 99, 117, 109, 101, 110, 116, 84, 105, 116, 108, 101, 7,
      0, 255, 184, 227, 157, 8, 0, 6, 4, 0, 255, 184, 227, 157, 8, 1, 1, 97, 0,
    ])

    const hex = Buffer.from(
      binaryData.buffer,
      binaryData.byteOffset,
      binaryData.length
    ).toString('hex')

    hexValidator(hex) is false

Environment:

  • OS: Mac
  • GraphQL Scalars Version: 1.20.1
@Urigo
Copy link
Owner

Urigo commented Mar 22, 2023

Hi @martinboksa and thank you for the report

Sorry but I'm not adding a lot here but just labeling it according to our new Contribution Guide and issue flow.

It seems already got into stage 1 thanks to your reproduction! Thank you for that!

Now in order to advance to stage 2 we'll need a failing test, would be great if someone could help progress the issues through the stages.

Thank you and sorry that this comment is not a complete solution (yet).

@Urigo Urigo added the stage/1-reproduction A reproduction exists label Mar 22, 2023
@martinboksa
Copy link
Author

martinboksa commented Mar 23, 2023

By the way, it's because the validator parses the hex string into small slices, and then fn parseInt removes all 'left zeroes' from each slice.

@theguild-bot theguild-bot mentioned this issue Apr 24, 2023
37 tasks
@cinan
Copy link

cinan commented May 11, 2023

we're using yarn patch as a (rather simplistic) workaround:

diff --git a/cjs/scalars/Byte.js b/cjs/scalars/Byte.js
index e6d3be273a4c77b9e19da28d3cdf3d80d0c75987..beb3936ccd611c92c6bad22c9a3d27daa2d095b7 100644
--- a/cjs/scalars/Byte.js
+++ b/cjs/scalars/Byte.js
@@ -5,18 +5,8 @@ const graphql_1 = require("graphql");
 const error_js_1 = require("../error.js");
 const base64Validator = /^(?:[A-Za-z0-9+\/]{4})*(?:[A-Za-z0-9+\/]{2}==|[A-Za-z0-9+\/]{3}=)?$/;
 function hexValidator(value) {
-    // Ensure that any leading 0 is removed from the hex string to avoid false negatives.
-    const sanitizedValue = value.charAt(0) === '0' ? value.slice(1) : value;
-    // For larger strings, we run into issues with MAX_SAFE_INTEGER, so split the string
-    // into smaller pieces to avoid this issue.
-    if (value.length > 8) {
-        let parsedString = '';
-        for (let startIndex = 0, endIndex = 8; startIndex < value.length; startIndex += 8, endIndex += 8) {
-            parsedString += parseInt(value.slice(startIndex, endIndex), 16).toString(16);
-        }
-        return parsedString === sanitizedValue;
-    }
-    return parseInt(value, 16).toString(16) === sanitizedValue;
+    const regexp = /^[0-9a-fA-F]+$/;
+    return regexp.test(value)
 }
 function validate(value, ast) {
     if (typeof value !== 'string' && !(value instanceof global.Buffer)) {

@ardatan
Copy link
Collaborator

ardatan commented May 11, 2023

Feel free to create a PR for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/1-reproduction A reproduction exists
Projects
None yet
Development

No branches or pull requests

4 participants