Skip to content

Fix issue with Content-Length header being calculated incorrectly - #28

Closed
ezekg wants to merge 1 commit into
BlockScore:masterfrom
sfoxhq:fix/content-length-header
Closed

Fix issue with Content-Length header being calculated incorrectly#28
ezekg wants to merge 1 commit into
BlockScore:masterfrom
sfoxhq:fix/content-length-header

Conversation

@ezekg

@ezekg ezekg commented Oct 2, 2017

Copy link
Copy Markdown
Contributor

As mentioned in #25, the Content-Length header is not calculated correctly for strings containing certain unicode characters. Here's a summary of the issue:

… the issue here is that the blockscore-node package is relying on String.length to determine the Content-Length header but the header needs to be the byte length, not string length, e.g. 'Zürich' is 7 bytes, not 6. This package should use Buffer.byteLength() instead of String.length to determine the content length header.

For example, this request fails: (note Content-Length: 345)

curl -X POST https://api.blockscore.com/candidates \
  -u "$BLOCKSCORE_API_KEY:" \
  -H 'Accept: application/vnd.blockscore+json;version=4' \
  -H 'Content-Type: application/json' \
  -H 'Content-Length: 345' \
  -d '{"birth_day":"xx","birth_month":"xx","birth_year":"xxxx","document_value":"Fxxxxxxx","document_type":"passport","name_first":"First","name_middle":"","name_last":"Last","address_street1":"Weststrasse xxx","address_city":"Zürich","address_subdivision":"ZH","address_postal_code":"xxxx","address_country_code":"CH","phone_number":"00xxxxxxxxxxxx"}'

with the response,

{
  "error": {
    "type": "invalid_request_error",
    "message": "Poorly formed JSON request body."
  }
}

while this request succeeds: (note Content-Length: 346)

curl -X POST https://api.blockscore.com/candidates \
  -u "$BLOCKSCORE_API_KEY:" \
  -H 'Accept: application/vnd.blockscore+json;version=4' \
  -H 'Content-Type: application/json' \
  -H 'Content-Length: 346' \
  -d '{"birth_day":"xx","birth_month":"xx","birth_year":"xxxx","document_value":"Fxxxxxxx","document_type":"passport","name_first":"First","name_middle":"","name_last":"Last","address_street1":"Weststrasse xxx","address_city":"Zürich","address_subdivision":"ZH","address_postal_code":"xxxx","address_country_code":"CH","phone_number":"00xxxxxxxxxxxx"}'

Closes #25.

@ezekg

ezekg commented Oct 2, 2017

Copy link
Copy Markdown
Contributor Author

Tests are also failing locally on master, so let me know if I need to do anything on that front.

@dgollahon

Copy link
Copy Markdown
Contributor

Hey, @ezekg, thanks for the fix! Sorry for the oversight on our part (and the flaky CI).

I pulled your branch and rebased it so you wouldn't have to do anything else to get the fix in. Closing this PR and using #30 instead since I don't have access to your fork. :)

@dgollahon dgollahon closed this Oct 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants