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

expires field in POST v2/subscriptions API should not allow the datetime before current time #4541

Closed
ArqamFarooqui110719 opened this issue May 9, 2024 · 12 comments
Labels
Milestone

Comments

@ArqamFarooqui110719
Copy link
Contributor

Bug description
I have run POST v2/subscriptions API with older datetime in expires field, and subscription created successfully (subscription should be created with older expires datetime value):

  • Orion version: 3.12

How to reproduce it
Steps to reproduce the behavior (as an example)

  1. Create an Subscription with any older expires field value, for example 2010-01-01T14:00:00.00Z:
    curl -v 'http://localhost:1026/v2/subscriptions' -H 'Content-Type: application/json' -d @- <<EOF
    {
        "description": "A subscription to get info about Room1",
        "subject": {
           "entities": [
             {
                 "id": "Room1",
                 "type": "Room"
             }
           ], 
           "condition": {
              "attrs": [
                 "pressure"
              ]
           }
        },
        "notification": {
           "http": {
              "url": "http://localhost:1028/accumulate"
           },
          "attrs": [
             "pressure"
          ]
       },
       "expires": "2010-01-01T14:00:00.00Z"
    }
    EOF
  2. Subscription created successfully i.e. wrong behavior ideally.

Expected behavior
subscription should not be created, and a clear message should be returned to user, something like "invalid/older expires field date value, Correct the expires datetime in your subscription"

@ArqamFarooqui110719
Copy link
Contributor Author

Hi @fgalan ,
Please let me know your opinion on this. If my understanding is correct, I'd like to work on this issue.

@fgalan
Copy link
Member

fgalan commented May 9, 2024

I think the current behaviour is correct, as long as the status of the created subscription (get GET /v2/subscription/{id} on that subscription) be expired (otherwise it would be a bug).

It would be weird than an user created a subscription with expiration date in the past but nothing in the Orion API (https://github.com/telefonicaid/fiware-orion/blob/master/doc/manuals/orion-api.md) precludes that as far as I remember (please tell me otherwise).

From my opinion, this issue should be closed, as it is expected behaviour.

@ArqamFarooqui110719
Copy link
Contributor Author

ArqamFarooqui110719 commented May 9, 2024

It would be weird than an user created a subscription with expiration date in the past but nothing in the Orion API (https://github.com/telefonicaid/fiware-orion/blob/master/doc/manuals/orion-api.md) precludes that as far as I remember (please tell me otherwise).

Yes. you are right.
I thought that even though it's weird behavior from user but we should prevent it. (The creation of new subscription with already expired datetime in expires field).

@ArqamFarooqui110719
Copy link
Contributor Author

There is another observation I had:
if we create subscription with invalid date in expires field, then also it creates the subscriptions, for example:
API with invalid expires datetime value:

curl -v localhost:1026/v2/subscriptions -s -S -H 'Content-Type: application/json' -d @- <<EOF
{
  "description": "A subscription to get info about Room1",
  "subject": {
    "entities": [
      {
        "id": "Room1",
        "type": "Room"
      }
    ],
    "condition": {
      "attrs": [
        "pressure"
      ]
    }
  },
  "notification": {
    "http": {
      "url": "http://localhost:1028/accumulate"
    },
    "attrs": [
      "pressure"
    ]
  },
  "expires": "2050-67-50T14:00:00.00Z"
}
EOF

Other invalid values of expires field which are working:

"expires": "2050-67-50T44:00:00.00Z"
"expires": "2050-67-50T44:00:0x.00Z"
"expires": "2050-04-05T14:00:0x.00Z"

@fgalan
Copy link
Member

fgalan commented May 10, 2024

There is another observation I had: if we create subscription with invalid date in expires field, then also it creates the subscriptions, for example: API with invalid expires datetime value:

curl -v localhost:1026/v2/subscriptions -s -S -H 'Content-Type: application/json' -d @- <<EOF
{
  "description": "A subscription to get info about Room1",
  "subject": {
    "entities": [
      {
        "id": "Room1",
        "type": "Room"
      }
    ],
    "condition": {
      "attrs": [
        "pressure"
      ]
    }
  },
  "notification": {
    "http": {
      "url": "http://localhost:1028/accumulate"
    },
    "attrs": [
      "pressure"
    ]
  },
  "expires": "2050-67-50T14:00:00.00Z"
}
EOF

Other invalid values of expires field which are working:

"expires": "2050-67-50T44:00:00.00Z"
"expires": "2050-67-50T44:00:0x.00Z"
"expires": "2050-04-05T14:00:0x.00Z"

That one seems to be a buggy behaviour. Invalided dates should not be allowed in any place.

What do you get when you do a GET /v2/subscriptions/{id} in the just created subscription with a wrong date in expiration?

@ArqamFarooqui110719
Copy link
Contributor Author

Example-1: Subscription

curl -v localhost:1026/v2/subscriptions -s -S -H 'Content-Type: application/json' -d @- <<EOF
{
  "description": "A subscription to get info about Room1",
  "subject": {
    "entities": [
      {
        "id": "Room1",
        "type": "Room"
      }
    ],
    "condition": {
      "attrs": [
        "pressure"
      ]
    }
  },
  "notification": {
    "http": {
      "url": "http://localhost:1028/accumulate"
    },
    "attrs": [
      "pressure"
    ]
  },
  "expires": "2010-43-57T32:00:00.00Z"
}
EOF

GET API Response: curl -v localhost:1026/v2/subscriptions/6641ee9b543308ac450f11e4

{"id":"6641ee9b543308ac450f11e4","description":"A subscription to get info about Room1","expires":"2013-08-27T08:00:00.000Z","status":"expired","subject":{"entities":[{"id":"Room1","type":"Room"}],"condition":{"attrs":["pressure"],"notifyOnMetadataChange":true}},"notification":{"attrs":["pressure"],"onlyChangedAttrs":false,"attrsFormat":"normalized","http":{"url":"http://localhost:1028/accumulate"},"covered":false}}

Example-2: Subscription

curl -v localhost:1026/v2/subscriptions -s -S -H 'Content-Type: application/json' -d @- <<EOF
{
  "description": "A subscription to get info about Room1",
  "subject": {
    "entities": [
      {
        "id": "Room1",
        "type": "Room"
      }
    ],
    "condition": {
      "attrs": [
        "pressure"
      ]
    }
  },
  "notification": {
    "http": {
      "url": "http://localhost:1028/accumulate"
    },
    "attrs": [
      "pressure"
    ]
  },
  "expires": "2022-53-67T32:66:13.00Z"
}
EOF

GET API Response: curl -v localhost:1026/v2/subscriptions/6641eefe543308ac450f11e5

{"id":"6641eefe543308ac450f11e5","description":"A subscription to get info about Room1","expires":"2026-07-07T09:06:13.000Z","status":"active","subject":{"entities":[{"id":"Room1","type":"Room"}],"condition":{"attrs":["pressure"],"notifyOnMetadataChange":true}},"notification":{"attrs":["pressure"],"onlyChangedAttrs":false,"attrsFormat":"normalized","http":{"url":"http://localhost:1028/accumulate"},"covered":false}}

@ArqamFarooqui110719
Copy link
Contributor Author

@fgalan Please provide your opinion on this. Thanks:)

@fgalan
Copy link
Member

fgalan commented May 16, 2024

So

  • Sub created with 2010-43-57T32:00:00.00Z (wrong), and 2013-08-27T08:00:00.000Z is got
  • Sub created with 2022-53-67T32:66:13.00Z (wrong), and 2026-07-07T09:06:13.000Z is got

Something weird is happending... Orion should return a 400 Bad Request error when wrong dates are used in expires field.

Maybe you could have a look and solve it in a PR, please? It should be easy... please have a look to parseContextAtribute.cpp and how parse8601Time() is used to check for valid dates.

@ArqamFarooqui110719
Copy link
Contributor Author

Thanks for the confirmation.
I'm looking into the issue and will raise a PR.

@fgalan fgalan added this to the 4.1.0 milestone Jun 7, 2024
@fgalan
Copy link
Member

fgalan commented Jun 7, 2024

After merging PR #4554 I understand this issue should be closed, shouldn't it?

@ArqamFarooqui110719
Copy link
Contributor Author

yes

@ArqamFarooqui110719
Copy link
Contributor Author

Closed as fixed in PR #4554

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

No branches or pull requests

2 participants