-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Web API: Endpoint does not work when identifier contains space #7552
Comments
I looked a bit more into this and I'm getting contradicting information. In DB, Builder's name has no restrictions: buildbot/master/buildbot/db/model.py Line 672 in 82d0eaf
There is restriction on size or characters in the config either: buildbot/master/buildbot/config/builder.py Lines 52 to 60 in 82d0eaf
There is even tests that lead to a broken data-api: buildbot/master/buildbot/test/unit/config/test_builder.py Lines 119 to 122 in 82d0eaf
The only place that seam to put restriction on a builder's name is the data-api here: buildbot/master/buildbot/data/builders.py Line 99 in 82d0eaf
(which make it validate that a builder name's length it less than 70 and match this regex: ^[a-zA-Z_-][a-zA-Z0-9._-]*$
There is a mention of limiting the buildername length in #3957 to avoid breaking the UI. I updated my branch with an even clearer reproduction: master...tdesveaux:buildbot:tds/api_space_identifier_issue#diff-91b181695ef480960bcd9482cd44a4cd1408038053e86ec7924cedd71881871d Using the same buildername used in the config.test_builder.py unicode test (which is considered valid), accessing the builder by name with the data api fails. |
I setup a reproduction unit test here: https://github.com/tdesveaux/buildbot/tree/tds/api_space_identifier_issue
I found this issue while writing a script to get a Build's logs. Some of the steps names contained spaces and I was getting 404 when querying their log with
/builders/n:builderid/builds/n:build_number/steps/i:step_name/logs/i:log_slug/raw
.I also reproduced the issue by creating a master, and changing the Builder name from
runtests
torun tests /
.checkconfig
didn't complain, running the master and a build worked fine.However, a query on
/api/v2/builders/run+tests+%2F
results in{"error": "Invalid path: builders/run+tests+/"}
.It looks like the issue is due to the data api using the
Identifier
to validate the arguments, which does not allow spaces and other characters. ButBuilder.name
,BuildStep.name
(and probably others) which are used as identifiers are not validated at config time to bring up the issue.I'm unsure how to proceed to fix this.
Either, check that any field that can be used as an API identifier is valid, which can be tricky if some of them are renderered (fail checkconfig, log a warn at runtime?).
Or make the API accept identifiers which contains any string.
The text was updated successfully, but these errors were encountered: