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

Add validation for resource match mode #3329

Conversation

philippthun
Copy link
Member

If provided, the mode field (resource match object) must be a string. Without this fix a request to /v3/resource_matches with

  { .. "mode": 644 .. }

resulted in an UnknownError. Now the error message

  "array contains at least one resource with a non-string mode"

is returned instead.

Furthermore the format is checked and if the provided value is not a POSIX file permissions string, the following error message is returned:

  "array contains at least one resource with an incorrect mode"
  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

If provided, the 'mode' field (resource match object) must be a string.
Without this fix a request to /v3/resource_matches with

  { .. "mode": 644 .. }

resulted in an UnknownError. Now the error message

  "array contains at least one resource with a non-string mode"

is returned instead.

Furthermore the format is checked and if the provided value is not a
POSIX file permissions string, the following error message is returned:

  "array contains at least one resource with an incorrect mode"
Copy link
Contributor

@johha johha left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@philippthun philippthun merged commit 1750fec into cloudfoundry:main Jun 30, 2023
@schulzh
Copy link

schulzh commented Aug 24, 2023

This change breaks compatibility with the official CF Java Client, which sends the mode along with the file type (e.g. 100644). There are several testcases that explicitly test against this mode, e.g. https://github.com/cloudfoundry/cf-java-client/blob/main/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/DefaultApplicationsTest.java#L1611

@FloThinksPi
Copy link
Member

FloThinksPi commented Aug 24, 2023

Improvement in #3414
See analysis there.

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.

5 participants