-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Mashape Analytics URI bug #470
Comments
I added better debugging, so now it'll say |
@SGrondin did you deploy your fix? Because the log message is still the same. |
As of today, the error still says:
So there is no way to know what it really contains. |
Now I can see the URI error properly:
and
and
to give a few. |
Here are several values reported as invalid:
Any idea why? Maybe the space? I will check the validation code later. |
It is visibly the space (see the regex). I will encode the URI but lua-nginx only offers escape_uri which is meant for URI components, and destroys the URI, so that will probably have to be custom. |
On a second thought, I am not sure I should touch the URI at all. After all, it is the client who requested it without escaping it (and using |
this is more of a HAR format / JSON validation issue than it is Mashape analytics ... we could expand the allowed character set ... maybe actually surprised nginx even allows it? |
So does this issue bounce back to @SGrondin (or @kennethklee)? |
bounce back to you first, do you want to follow the RFC specs or not? |
It's not so much about the specs. The client did not respect the specs, we have to reflect it in the analytics logs. |
that I agree with, and I believe latest build has error logging output in |
No, I meant the logs stored by analytics. I mean on analytics' dashboard. |
but we are discarding the input if its invalid ... not storing anywhere, so no way to message the user about it ... is that what you mean? otherwise I'm not following |
I mean you should not discard it, and you should store it. |
that could become very dangerous since clients are paying for analytics data retention and storage, if somebody is sending malformed requests they could easily take over a user's quota and flood the system .. that said, this conversation is about Analytics then, so lets take the discussion there, unless there's something specific to Kong, otherwise, lets close this issue, |
They would not take more space because of accepting invalid URIs. They could as well flood the system with valid URIs? |
Moved to Mashape Analytics. |
actually, all of the examples provided above of the errors are because of the space in the URL, not the |
Damn it, ngx_lua returns those values unescaped. And the only way to escape them back is with |
I had the same problem with Node / PHP ... I end up parsing and re-constructing the URI every time to be safe. |
So while writing the serializer I respected the ALF schema (where Hence I used |
Fix invalid ALFs when the URI contains reserved characters because `request_uri` returns an unescaped string. `uri` returns the string untouched but with querystring, which is fine for HAR. Fix #470
* fix(proto): add the include directory * fix(bc): keep the change backwards compatible
The text was updated successfully, but these errors were encountered: