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

[rest] Fix ThingActionsResource does not accept new Thing action UIDs with hash #4445

Merged
merged 2 commits into from
Nov 11, 2024

Conversation

florian-h05
Copy link
Contributor

Regression from #4441.

… due to wrong path RegEx

Signed-off-by: Florian Hotze <[email protected]>
@florian-h05 florian-h05 requested a review from a team as a code owner November 10, 2024 20:44
@lolodomo
Copy link
Contributor

Probably correct

@Path("/{thingUID}/{actionUid: [a-zA-Z0-9]+\\.[a-zA-Z0-9]+}")
// accept actionUid in the form of "scope.actionTypeUid" or "scope.actionTypeUid#signatureHash"
// # is URL encoded as %23
@Path("/{thingUID}/{actionUid: [a-zA-Z0-9]+\\.[a-zA-Z0-9]+(%23[a-zA-Z0-9]+)?}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hash is a hex string, correct? In that case A-Fa-f0-9 would be sufficient. Since we do a lookup on the hash and that is case-sensitive it Coups be a good idea to also restrict that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, just implemented and pushed that.
Local test looks good.

Signed-off-by: Florian Hotze <[email protected]>
@J-N-K J-N-K added this to the 4.3 milestone Nov 11, 2024
@J-N-K J-N-K merged commit 31d3434 into openhab:main Nov 11, 2024
5 checks passed
@florian-h05 florian-h05 deleted the rest-thingaction-fix branch November 11, 2024 08:25
@florian-h05
Copy link
Contributor Author

Can someone please trigger a new snapshot build?

@kaikreuzer
Copy link
Member

kaikreuzer commented Nov 11, 2024

It's already done: https://ci.openhab.org/job/openHAB-Core/58/changes (for core that is - not yet the distro).

@kaikreuzer
Copy link
Member

Now there's also a new distro for testing: https://ci.openhab.org/job/openHAB-Distribution/4374/

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

Successfully merging this pull request may close these issues.

4 participants