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

fix: media upload to have applicationName as User-Agent #2227

Merged
merged 4 commits into from
Jan 20, 2023

Conversation

suztomo
Copy link
Member

@suztomo suztomo commented Jan 20, 2023

Fixes #2222

  • Upload to have applicationName as User-Agent
  • request initializer to set user-agent header

How I confirmed the fix:

I locally installed this branch via mvn install and specified the SNAPSHOT version of the BOM:

  <dependencyManagement>
    <dependencies>
      <dependency>
        <groupId>com.google.api-client</groupId>
        <artifactId>google-api-client-bom</artifactId>
        <version>2.1.3-SNAPSHOT</version>
        <type>pom</type>
        <scope>import</scope>
      </dependency>
    </dependencies>
  </dependencyManagement>

  <dependencies>
    <dependency>
      <groupId>com.google.apis</groupId>
      <artifactId>google-api-services-storage</artifactId>
      <version>v1-rev20220705-2.0.0</version>
    </dependency>
...

I ran @BenWhitehead 's reproducer (with my small modification):

package org.example;

import static sun.net.www.protocol.http.HttpURLConnection.userAgent;

import com.google.api.client.googleapis.GoogleUtils;
import com.google.api.client.googleapis.media.MediaHttpUploader;
import com.google.api.client.http.GenericUrl;
import com.google.api.client.http.HttpHeaders;
import com.google.api.client.http.HttpRequest;
import com.google.api.client.http.HttpRequestFactory;
import com.google.api.client.http.HttpRequestInitializer;
import com.google.api.client.http.HttpResponse;
import com.google.api.client.http.InputStreamContent;
import com.google.api.client.http.javanet.NetHttpTransport;
import com.google.api.client.json.JsonFactory;
import com.google.api.client.json.gson.GsonFactory;
import com.google.api.services.storage.Storage;
import com.google.api.services.storage.Storage.Objects.Insert;
import com.google.api.services.storage.model.StorageObject;
import com.google.auth.http.HttpCredentialsAdapter;
import com.google.auth.oauth2.GoogleCredentials;
import java.io.ByteArrayInputStream;
import java.nio.charset.StandardCharsets;

public class App 
{
    // Run with VM option to have -Djava.util.logging.config.file=src/main/resources/logging.properties
    public static void main( String[] args ) throws Exception {
        GoogleCredentials credentials = GoogleCredentials.getApplicationDefault();
        String userAgent = "Foo/1.0 (Bar:Baz/1.0) Tomo/1.0";
        HttpCredentialsAdapter credentialsInitializer = new HttpCredentialsAdapter(credentials);

        NetHttpTransport transport = new NetHttpTransport.Builder()
            .trustCertificates(GoogleUtils.getCertificateTrustStore())
            .build();
        HttpRequestFactory rf = transport.createRequestFactory();
        JsonFactory jsonFactory = new GsonFactory();

        Storage storage = new Storage.Builder(rf.getTransport(), jsonFactory, credentialsInitializer)
            .setApplicationName(userAgent)
            .build();


        // Project suztomo-terraform-4f9e48 has this bucket.
        String bucket = "suztomo_test_b252963877"; // TODO: replace with your bucket

        StorageObject storageObject = new StorageObject().setName("temp.txt").setBucket(bucket);
        byte[] bytes = "hi".getBytes(StandardCharsets.UTF_8);
        ByteArrayInputStream bais = new ByteArrayInputStream(bytes);
        InputStreamContent content = new InputStreamContent("text/plain", bais);

        Insert insert = storage.objects().insert(bucket, storageObject, content);
        GenericUrl url = insert.buildHttpRequestUrl();
        MediaHttpUploader mediaHttpUploader = insert.getMediaHttpUploader();

        HttpResponse upload = mediaHttpUploader.upload(url);
        System.out.println("Upload response code: " + upload.getStatusCode() + " " + upload.getStatusMessage());
    }
}

Note that userAgent is used in only setApplicationName(userAgent).

I placed src/main/resources/logging.properties so that it prints debug (FINE-level) log:

handlers = java.util.logging.ConsoleHandler
java.util.logging.ConsoleHandler.level = FINE

# Set up logging of HTTP requests and responses (uncomment "level" to show)
com.google.api.level = FINE

This is specified via -Djava.util.logging.config.file=src/main/resources/logging.properties argument in IntelliJ.

It printed out the expected User-Agent when uploading the data:

...
Jan 20, 2023 10:59:01 AM com.google.api.client.http.HttpRequest execute
CONFIG: -------------- REQUEST  --------------
PUT https://storage.googleapis.com/upload/storage/v1/b/suztomo_test_b252963877/o?uploadType=resumable&upload_id=ADPycdstG5t6ppvQpO9MoZROy-k67Qonkt_EFy8fUv5NQOW6hODYzBs8T3iEBy3UD9qy4xz25pnrOc41j_te7WPdu4ZG
Accept-Encoding: gzip
Authorization: <Not Logged>
Content-Range: bytes 0-1/2
User-Agent: Foo/1.0 (Bar:Baz/1.0) Tomo/1.0 Google-HTTP-Java-Client/1.42.3 (gzip)
x-goog-user-project: suztomo-terraform-4f9e48
...

@suztomo suztomo requested a review from a team as a code owner January 20, 2023 15:45
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Jan 20, 2023
@suztomo suztomo changed the title Media Upload to have applicationName as User-Agent fix: media upload to have applicationName as User-Agent Jan 20, 2023
@lqiu96
Copy link
Contributor

lqiu96 commented Jan 20, 2023

LGTM -- Changes make sense. I do that the setApplicationName -> User Agent header is documented here:

/**
* Sets the application name to be used in the UserAgent header of each request or {@code null}
* for none.
*
* <p>Overriding is only supported for the purpose of calling the super implementation and
* changing the return type, but nothing else.
*/
public Builder setApplicationName(String applicationName) {
this.applicationName = applicationName;
return this;
}
inside AbstractGoogleClient. I think this is searchable on CloudRAD, but is hidden away in the Builder: https://cloud.google.com/java/docs/reference/google-api-client/latest/com.google.api.client.googleapis.services.AbstractGoogleClient.Builder#com_google_api_client_googleapis_services_AbstractGoogleClient_Builder_setApplicationName_java_lang_String_

Do you think it would make sense to have this documented a bit more visible?

@suztomo
Copy link
Member Author

suztomo commented Jan 20, 2023

is hidden away in the Builder

I see it in IntelliJ. Where do you wish you want to see it?

Screenshot 2023-01-20 at 11 45 23 AM

@lqiu96
Copy link
Contributor

lqiu96 commented Jan 20, 2023

I see it in IntelliJ.

Yeah, I think it's fine as it is. Personally, I would have assumed setting the user-agent would be something like setUserAgent() rather than setApplicationName(). Though on second thought, it would make sense for the user agent to be the application name (I guess point being is that from a completely new perspective, it may not have clicked immediately).

I do see there are guides here: https://googleapis.github.io/google-api-java-client/media-upload.html. Maybe a quick blurb/ section/ example on this page about how applicationName is the user-agent that is passed in the headers -- or if you think it's already clear enough I'm fine with as well.

Comment on lines +354 to 357
String applicationName = abstractGoogleClient.getApplicationName();
HttpRequestInitializer requestInitializer =
mediaUploadRequestUserAgentInitializer(applicationName, requestFactory.getInitializer());
this.uploader =
Copy link
Member Author

Choose a reason for hiding this comment

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

Special handling for applicationName,
Screenshot 2023-01-20 at 9 49 39 AM

@suztomo
Copy link
Member Author

suztomo commented Jan 20, 2023

@BenWhitehead Would you review this fix? The pull request description has how I confirmed it with regard to the internal bug b/252963877.

@suztomo suztomo merged commit 2595de0 into googleapis:main Jan 20, 2023
@suztomo suztomo deleted the i2222 branch January 20, 2023 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AbstractGoogleClientRequest.initializeMediaUpload is not using applicationName to User-Agent HTTP header
3 participants