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

[BUG] in POST v2/subscriptions with "q" condition having invalid chars #1994

Closed
iariasleon opened this issue Apr 4, 2016 · 8 comments
Closed
Assignees
Milestone

Comments

@iariasleon
Copy link
Contributor

in POST v2/subscriptions with "q" condition having invalid chars

dataset

      | q           |
      |-------------|
      | house<flat> |
      | house=flat  |
      | house"flat" |
      | house'flat' |
      | house;flat  |
      | house(flat) |
      | house_?     |
      | house_&     |
      | house_/     |
      | house_#     |
      | my house    |

subscription request

POST http://localhost:1026/v2/subscriptions
Content-Type: application/json
Fiware-Service: test_condition_expression_error
Fiware-ServicePath: /test
{"notification": {"callback": "http://localhost:1234", "attributes": ["temperature"]}, "expires": "2016-04-05T14:00:00.00Z", "subject": {"entities": [{"idPattern": ".*", "type": "room"}], "condition": {"attributes": ["temperature"], "expression": {"q": "house;flat"}}}}

subscription response

http code: 201
date: Mon, 04 Apr 2016 10:50:40 GMT
connection: Keep-Alive
content-length: 0
location: /v2/subscriptions/570247000f6b2ac1b6346f02

expected response

http code: 400
{"error":"BadRequest","description":"Invalid characters in condition expression"}
@iariasleon
Copy link
Contributor Author

iariasleon commented Apr 29, 2016

In the version:

  "version" : "1.0.0-next"
  "git_hash" : "fee3d035442e78c1a2e19720f68bdeba083ca560",

And they respond:
Only in this case house=flat, it returns a payload malformed (only description) and 200 in http code.

http code: 200
date: Fri, 29 Apr 2016 12:02:28 GMT
fiware-correlator: 3ecb594a-0e02-11e6-bf64-005056a20feb
connection: Keep-Alive
content-type: application/json
content-length: 40
invalid character found in URI param /q/

@iariasleon
Copy link
Contributor Author

In CB version:

 "version" : "1.1.0-next",
  "git_hash" : "301c258a9ef8f05cabdc276b7e21365e8f4d5913"

These cases respond correctly, with a json object and the description is invalid character found in URI param /q/

      | q           |
      |-------------| 
      | house=flat  |
      | house"flat" |
      | house'flat' |    
      | house(flat) |     

But these case are responding 201 - OK

      | q           |
      |-------------|
      | house<flat> | 
      | house;flat  |    
      | house_?     |
      | house_&     |
      | house_/     |
      | house_#     |
      | my house    |

@iariasleon
Copy link
Contributor Author

Re-tested in the CB version. It issue still fails and returns 201-Created

  "version" : "1.2.0-next",
  "git_hash" : "d81dcf28e71d8d63dc0929ef7a6b73d7db47421b"

@kzangeli
Copy link
Member

kzangeli commented Jan 16, 2017

These are the forbidden chars as of right now (Jan 2007):

inline static bool commonForbidden(char c)
{
  switch (c)
  {
  case '<':
  case '>':
  case '"':
  case '\'':
  case '=':
  case ';':
  case '(':
  case ')':
    return true;
  }
  return false;
}

From this set, chars can be excluded.

Now, house<flat>, that is broken up as house < flat> contains the forbidden char >, but, house_? contains no forbidden chars, neither any of the other strings in the final list, except house;flat.

This latter case in a bit special as ; is the field separator inside q-string filters.
q='house;flat' means existence of attr house OR flat (I'm a bit rusty about string filters, but I think this is what it means ...).

Why is '&' and '?' not forbidden chars?
Probably because they're separators in the URI.
However, we check forbidden chars long after breaking the URI in components and sub components ...
Perhaps we need to see over this and include '&' and '?' as forbidden characters ... ?

@fgalan
Copy link
Member

fgalan commented Jan 16, 2017

Fixed by PR #2817. Please @iariasleon have a look when the PR gets merged (remember to move the two cases we have discussed at skype to the "happy case").

@iariasleon
Copy link
Contributor Author

iariasleon commented Jan 18, 2017

re-tested in:

  "version" : "1.6.0-next",  
  "git_hash" : "b21859097fea00ca5fd83514897cb95901115b0d",

Only fail this case {"q": "house'flat'==34"} returning a 201 - Created instead of a 400 - Bad Request

@fgalan
Copy link
Member

fgalan commented Jan 19, 2017

Fixes in PR #2826. @iariasleon , you can check again, pls.

@iariasleon
Copy link
Contributor Author

LGTM

                    SUMMARY:
 -------------------------------------------------------------------------------------------------------------------------------------------------------------------
|                                                   name                                                        | passed | failed | skipped |  total  | duration(s) |
 -------------------------------------------------------------------------------------------------------------------------------------------------------------------
| components\ngsiv2\subscriptions\create_a_new_subscription\create_a_new_subscription_subject_condition.feature |      9 |      0 |     249 |     258 |       0.588 |
 -------------------------------------------------------------------------------------------------------------------------------------------------------------------
|  Date: Thursday, January 19, 2017 15:55:57
 -------------------------------------------------
1 feature passed, 0 failed, 0 skipped
9 scenarios passed, 0 failed, 249 skipped
45 steps passed, 0 failed, 1335 skipped, 0 undefined
Took 0m0.588s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants