Skip to content

Conversation

@claesjac
Copy link

This PR adds a canonicalized version of the QueryString and includes that in the signature generated.

@kaltura-hooks
Copy link

Hi @claesjac,
Thank you for contributing this pull request!
Please sign the Kaltura CLA so we can review and merge your contribution.
Learn more at http://bit.ly/KalturaContrib

@erankor
Copy link
Collaborator

erankor commented Dec 21, 2019

I'm sorry, this patch does not conform to the coding standards of this module.
A few things that I'm seeing -

  1. It seems to be doing more work than needed in several cases - e.g.
    a. I don't see why there's a need to copy the key/values
    b. qs + qs_len calculated multiple times instead of just once
    c. Both qs & qs_len are updated each time instead of updating only qs + using the end pos
    d. Use of ngx_array_create while ngx_array_init could have worked
  2. There is no handling for allocation errors
  3. There are style issues that violate nginx coding standards - e.g. 80 chars width, function opening brace in new line etc.

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.

3 participants