feat: disable dynamic code loading properties by default #2606
+298
−139
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Disable Connection URL properties that dynamically invoke code by default. These properties can now only be used if the corresponding System property has been enabled. This gives users control over whether they want to enable these properties in their applications or not. You should only enable the use of these properties as long as untrusted end users cannot dynamically set these. That is; If your application is a public service that allows end users to set a Connection URL, then you should not enable the use of these properties, as it would allow a user to try to specify a credentials or channel provider class that is not a valid provider. The constructor of the selected class would in that case still be invoked.
product-auto-label
bot
added
size: l
manu2
reviewed
Sep 7, 2023
There was a problem hiding this comment.
Should we add a test for WithoutEnablingProperty() that checks the exception message for credentialsProvider as well?
There was a problem hiding this comment.
We test that here: https://github.com/googleapis/java-spanner/pull/2606/files#r1318465848
olavloite
commented
Sep 7, 2023
| SpannerException.class, () -> ConnectionOptions.newBuilder().setUri(uri).build()); | ||
| assertEquals(ErrorCode.FAILED_PRECONDITION, exception.getErrorCode()); | ||
| assertEquals( | ||
| "FAILED_PRECONDITION: credentialsProvider can only be used if the system property ENABLE_CREDENTIALS_PROVIDER has been set to true. " |
There was a problem hiding this comment.
The error that you get when setting a CredentialsProvider without enabling the property is verified here.
manu2
approved these changes
Sep 8, 2023
gcf-merge-on-green bot
pushed a commit
that referenced
this pull request
Sep 12, 2023
🤖 I have created a release *beep* *boop* --- ## [6.47.0](https://togithub.com/googleapis/java-spanner/compare/v6.46.0...v6.47.0) (2023-09-12) ### Features * Add devcontainers for enabling github codespaces usage. ([#2605](https://togithub.com/googleapis/java-spanner/issues/2605)) ([a7d60f1](https://togithub.com/googleapis/java-spanner/commit/a7d60f13781f87054a1631ca511492c5c8334751)) * Disable dynamic code loading properties by default ([#2606](https://togithub.com/googleapis/java-spanner/issues/2606)) ([d855ebb](https://togithub.com/googleapis/java-spanner/commit/d855ebbd2dec11cdd6cdbe326de81115632598cd)) ### Bug Fixes * Add reflection configurations for com.google.rpc classes ([#2617](https://togithub.com/googleapis/java-spanner/issues/2617)) ([c42460a](https://togithub.com/googleapis/java-spanner/commit/c42460ae7b6bb5874cc18c7aecff34186dcbff2a)) * Avoid unbalanced session pool creation ([#2442](https://togithub.com/googleapis/java-spanner/issues/2442)) ([db751ce](https://togithub.com/googleapis/java-spanner/commit/db751ceebc8b6981d00cd07ce4742196cc1dd50d)) ### Dependencies * Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.15.0 ([#2615](https://togithub.com/googleapis/java-spanner/issues/2615)) ([ac762fb](https://togithub.com/googleapis/java-spanner/commit/ac762fbf079db79eab5f2ebee971b850ac89eb11)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
surbhigarg92
pushed a commit
to surbhigarg92/java-spanner
that referenced
this pull request
Oct 5, 2023
🤖 I have created a release *beep* *boop* --- ## [6.47.0](https://togithub.com/googleapis/java-spanner/compare/v6.46.0...v6.47.0) (2023-09-12) ### Features * Add devcontainers for enabling github codespaces usage. ([googleapis#2605](https://togithub.com/googleapis/java-spanner/issues/2605)) ([a7d60f1](https://togithub.com/googleapis/java-spanner/commit/a7d60f13781f87054a1631ca511492c5c8334751)) * Disable dynamic code loading properties by default ([googleapis#2606](https://togithub.com/googleapis/java-spanner/issues/2606)) ([d855ebb](https://togithub.com/googleapis/java-spanner/commit/d855ebbd2dec11cdd6cdbe326de81115632598cd)) ### Bug Fixes * Add reflection configurations for com.google.rpc classes ([googleapis#2617](https://togithub.com/googleapis/java-spanner/issues/2617)) ([c42460a](https://togithub.com/googleapis/java-spanner/commit/c42460ae7b6bb5874cc18c7aecff34186dcbff2a)) * Avoid unbalanced session pool creation ([googleapis#2442](https://togithub.com/googleapis/java-spanner/issues/2442)) ([db751ce](https://togithub.com/googleapis/java-spanner/commit/db751ceebc8b6981d00cd07ce4742196cc1dd50d)) ### Dependencies * Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.15.0 ([googleapis#2615](https://togithub.com/googleapis/java-spanner/issues/2615)) ([ac762fb](https://togithub.com/googleapis/java-spanner/commit/ac762fbf079db79eab5f2ebee971b850ac89eb11)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Disable Connection URL properties that dynamically invoke code by default. These properties can now only be used if the corresponding System property has been enabled. This gives users control over whether they want to enable these properties in their applications or not.
You should only enable the use of these properties as long as untrusted end users cannot dynamically set these. That is; If your application is a public service that allows end users to set a Connection URL, then you should not enable the use of these properties, as it would allow a user to try to specify a credentials or channel provider class that is not a valid provider. The constructor of the selected class would in that case still be invoked.