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

feat: disable dynamic code loading properties by default #2606

Merged
merged 2 commits into from
Sep 11, 2023

Conversation

olavloite
Copy link
Collaborator

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.

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.
@olavloite olavloite requested a review from arpan14 August 31, 2023 13:03
@olavloite olavloite requested a review from a team as a code owner August 31, 2023 13:03
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner API. labels Aug 31, 2023
@olavloite olavloite requested a review from manu2 September 1, 2023 12:35
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we add a test for WithoutEnablingProperty() that checks the exception message for credentialsProvider as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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. "
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The error that you get when setting a CredentialsProvider without enabling the property is verified here.

@olavloite olavloite requested a review from manu2 September 7, 2023 11:27
@olavloite olavloite merged commit d855ebb into main Sep 11, 2023
24 checks passed
@olavloite olavloite deleted the disable-connection-properties branch September 11, 2023 11:12
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
Labels
api: spanner Issues related to the googleapis/java-spanner API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants