search

pact-foundation / pact-reference

Reference implementations for the pact specifications
https://pact.io
MIT License
93 stars 46 forks source link

Pact_verifier: Tags in URL must be made URL safe #166

Closed tugtugtug closed 2 years ago

tugtugtug commented 2 years ago

Cargo.toml

[dependencies]
pact_mock_server = "0.7"
pact_mock_server_ffi = "0.1"
pact_matching = "0.9"
pact_verifier = "0.10.9"
pact_models = "0.0"
pact_ffi = "0.0"

I have already reported the issue in pact-js (https://github.com/pact-foundation/pact-js/issues/768), but since the source of the error belongs here, and the issue I reported in pact-js is not moving, I'm replicating the ticket here for some awareness.

This is a regression from the ruby version of the verifier. With the Ruby version, the tags are properly made URL safe when posting to the broker. However, with the rust verifier, we get the following error. The only difference seems to be the tag not escaped.

I am aware of the branch option introduced. But as a good practice, if tags need to be included in URLs, it shall be made URL safe.

[2021-11-30T15:18:49Z ERROR pact_verifier] Publishing of verification results failed with an error: Error with the content of a HAL resource - Request to pact broker URL 'https://broker/pacticipants/test%40project/versions/d80c7b3/tags/branch/master' failed - HTTP status client error (404 Not Found) for url (https://broker/pacticipants/test%40project/versions/d80c7b3/tags/branch/master)
mefellows commented 2 years ago

Thanks @tugtugtug. The current issue is that our NodeJS beta project is broken (Node 16 builds are failing on a downstream dependency). So yes, this needs to be fixed, but even if it's fixed here it won't make it into the beta release cycle until we address the other build I'm afraid.

tugtugtug commented 2 years ago

Thanks @mefellows , wasn't aware of that failure, but definitely good to know as the reason for the delay. But regardless, this is still a bug here in the rust lib, and needs to be addressed sooner or later. And hopefully by the time the beta build is fixed, we have a fixed rust lib to be upgraded to?

uglyog commented 2 years ago

I have released all the Rust libs with this fix

mefellows commented 2 years ago

Awesome, thanks Ron!

tugtugtug commented 2 years ago

Verified with the latest pactjs.beta.56. Thanks all for your help!