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

in POST v2/subscriptions if notification httpCustom url has invalid value the subsc is created #2280

Closed
iariasleon opened this issue Jun 15, 2016 · 9 comments
Assignees
Milestone

Comments

@iariasleon
Copy link
Contributor

iariasleon commented Jun 15, 2016

in POST v2/subscriptions if notification httpCustom url has invalid value the subsc is created

dataset used (replace these values into url field in request below)

      | url               |
      |-------------------|
      | ws://             |
      | ://localhost      |
      | http//localhost   |
      | http:/localhost   |
      | http:localhost    |
      | http://localhost: |
      | http://           |
      | //a               |
      | ///a              |
      | foo.com           |
      | rdar://1234       |
      | h://test          |
      | http://           |
      | ://               |
      | ftps://foo.bar/   |

Second dataset with the same behavior

      | url                        |
      |----------------------------| 
      | http://localhost:900000    |
      | http://localhost:dsfsdf    |
      | http://localhost\my_path   |
      | http://e34.56.45.34        |
      | http://34,56.45.34         |
      | http://34.56.45.34;3454    |
      | http://34.56:3454          |
      | http://.                   |
      | http://..                  |
      | http://../                 |
      | http://?                   |
      | http://??/                 |
      | http://#                   |
      | http://##/                 |
      | http://foo.bar/foo(bar)baz |
      | http://-error-.invalid/    |
      | http://a.b--c.de/          |
      | http://-a.b.co             |
      | http://a.b-.co             |
      | http://0.0.0.0             |
      | http://1.1.1.1.1           |
      | http://3441.2344.1231.1123 |
      | http://123.123.123         |
      | http://3628126748          |
      | http://.www.foo.bar/       |
      | http://www.foo.bar./       |
      | http://.www.foo.bar./      |

Third dataset:

      | url       |
      |-----------|
      | //        |
      | ///       |
      | http:///a |

subscription request

url: POST http://qa-orion-fe-02:1026/v2/subscriptions
headers:
    Content-Type: application/json
    Fiware-Service: test_notif_http_custom_url_error
    Fiware-ServicePath: /test
payload: {"notification": {"httpCustom": {"url": "ftps://foo.bar/"}, "attrs": ["temperature"]}, "expires": "2016-04-05T14:00:00.00Z", "subject": {"entities": [{"idPattern": ".*"}], "condition": {"attrs": ["temperature"]}}}

request response

http code: 201 - Created
headers:
   date: Wed, 15 Jun 2016 11:23:21 GMT
   fiware-correlator: 91559140-32eb-11e6-986e-005056a20feb
   connection: Keep-Alive
   content-length: 0
   location: /v2/subscriptions/576145a63ba0ad86fb27748b
payload:

mongo docs

{ "_id" : ObjectId("576145a53ba0ad86fb27747d"), "expiration" : NumberLong(1459864800), "reference" : "ws://", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb27747e"), "expiration" : NumberLong(1459864800), "reference" : "://localhost", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb27747f"), "expiration" : NumberLong(1459864800), "reference" : "http//localhost", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb277480"), "expiration" : NumberLong(1459864800), "reference" : "http:/localhost", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb277481"), "expiration" : NumberLong(1459864800), "reference" : "http:localhost", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb277482"), "expiration" : NumberLong(1459864800), "reference" : "http://localhost:", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb277483"), "expiration" : NumberLong(1459864800), "reference" : "http://", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb277484"), "expiration" : NumberLong(1459864800), "reference" : "//a", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb277485"), "expiration" : NumberLong(1459864800), "reference" : "///a", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb277486"), "expiration" : NumberLong(1459864800), "reference" : "foo.com", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb277487"), "expiration" : NumberLong(1459864800), "reference" : "rdar://1234", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb277488"), "expiration" : NumberLong(1459864800), "reference" : "h://test", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb277489"), "expiration" : NumberLong(1459864800), "reference" : "http://", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a53ba0ad86fb27748a"), "expiration" : NumberLong(1459864800), "reference" : "://", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }
{ "_id" : ObjectId("576145a63ba0ad86fb27748b"), "expiration" : NumberLong(1459864800), "reference" : "ftps://foo.bar/", "custom" : true, "throttling" : NumberLong(0), "servicePath" : "/test", "status" : "active", "entities" : [ { "id" : ".*", "isPattern" : "true" } ], "attrs" : [ "temperature" ], "blacklist" : false, "conditions" : [ { "type" : "ONCHANGE", "value" : [ "temperature" ] } ], "expression" : { "q" : "", "geometry" : "", "coords" : "", "georel" : "" }, "format" : "normalized" }

Expected response

http code: 400 - Bad Request
payload: {"error":"BadRequest","description":"Invalid URL parsing notification url"}
@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"

@fgalan
Copy link
Member

fgalan commented Jan 16, 2017

Fixed in PR #2815. Please @iariasleon check (once the PR gets merged).

@iariasleon
Copy link
Contributor Author

CB version used:

  "version" : "1.6.0-next",
  "git_hash" : "6232159b397f40aff97abd497b1e142a510e3e6d",

LGTM to the first and third datasets

http code: 400 - Bad Request
headers:
   Connection: Keep-Alive
   Content-Length: 59
   Content-Type: application/json
   Fiware-Correlator: 7d5f872c-dbf1-11e6-8d21-005056a20feb
   Date: Mon, 16 Jan 2017 13:41:31 GMT
payload: {"error":"BadRequest","description":"invalid custom /url/"}

But the second dataset continues to failing:

http code: 201 - Created
headers:
   Connection: Keep-Alive
   Content-Length: 0
   Location: /v2/subscriptions/587cce80364ee924a01b0bef
   Fiware-Correlator: 0efc4882-dbf2-11e6-8299-005056a20feb
   Date: Mon, 16 Jan 2017 13:45:36 GMT
payload:

@fgalan
Copy link
Member

fgalan commented Jan 16, 2017

@iariasleon pls clarify which url in the second dataset is the case causing the problem (there are around 30-40 individual urls in that dataset ;)?

@fgalan fgalan assigned kzangeli and unassigned iariasleon Jan 16, 2017
@iariasleon
Copy link
Contributor Author

All these wrong url are failing (They return a 201 instead of a 400):

See this scenario:
https://github.com/telefonicaid/fiware-orion/blob/master/test/acceptance/behave/components/ngsiv2/subscriptions/create_a_new_subscription/create_a_new_subscription_notification_http.feature#L477-L526

Second dataset with the same behavior

      | url                        |
      |----------------------------| 
      | http://localhost:900000    |
      | http://localhost:dsfsdf    |
      | http://localhost\my_path   |
      | http://e34.56.45.34        |
      | http://34,56.45.34         |
      | http://34.56.45.34;3454    |
      | http://34.56:3454          |
      | http://.                   |
      | http://..                  |
      | http://../                 |
      | http://?                   |
      | http://??/                 |
      | http://#                   |
      | http://##/                 |
      | http://foo.bar/foo(bar)baz |
      | http://-error-.invalid/    |
      | http://a.b--c.de/          |
      | http://-a.b.co             |
      | http://a.b-.co             |
      | http://0.0.0.0             |
      | http://1.1.1.1.1           |
      | http://3441.2344.1231.1123 |
      | http://123.123.123         |
      | http://3628126748          |
      | http://.www.foo.bar/       |
      | http://www.foo.bar./       |
      | http://.www.foo.bar./      |

@fgalan fgalan assigned iariasleon and unassigned kzangeli Jan 30, 2017
@fgalan
Copy link
Member

fgalan commented Jan 30, 2017

Could have been fixed by PR #2855. Please @iariasleon re-check.

Take into account that some of the examples you use (http://1.1.1.1.1) correspond actually to valid URLs when the string is interpreted as hostname and not ip.

@iariasleon
Copy link
Contributor Author

Re-tested and only these tests are failing:

      | http://-error-.invalid/                      |
      | http://-a.b.co                               |
      | http://a.b-.co                               |
      ...
      | https://localhost123456789012345678901234567890123456789012345678901234567890.my:1234                                                                                                                                                                                            |  

And about the reference used:

  • "Labels such as 2600 and 3abc may be used in hostnames, but -hi-, _hi_ and *h*i are invalid."
  • " Each label must be between 1 and 63 characters long and the entire hostname (including the delimiting dots but not a trailing dot) has a maximum of 253 ASCII characters"

@kzangeli
Copy link
Member

The function that verifies the validity of the hostnames isn't 100% correct.
Just a quick fix to improve the nothingness we had before.
What is missing is to split the hostname into 'items' ( item . item . item ...) and check that no item is more than 63 chars long and that they don't start or end in hyphen, something like that.

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