google-api-go-client icon indicating copy to clipboard operation
google-api-go-client copied to clipboard

storage: Incorrect page token value in the URL

Open karanpopat opened this issue 2 years ago • 5 comments

Environment details

  • Programming language: Go
  • Package version: go1.20.3 darwin/arm64

Steps to reproduce

package main

import (
	"context"
	"google.golang.org/api/googleapi"
	"google.golang.org/api/storage/v1
)

func main() {
	bucketName := "bucket-name"

	ctx := context.Background()

	// To get config arguments from plugin config file
	opts := setSessionConfig(ctx, d.Connection)

	// so it was not in cache - create service
	svc, err := storage.NewService(ctx, opts...)
	if err != nil {
		return nil, err
	}

	resp := svc.Objects.List(bucketName).Projection("full").MaxResults(100)
	if err := resp.Pages(ctx, func(page *storage.Objects) error {
		for _, object := range page.Items {

			plugin.Logger(ctx).Error("listStorageObjects", page.NextPageToken, page.Header)

			if d.RowsRemaining(ctx) == 0 {
				page.NextPageToken = ""
				break
			}
		}
		return nil
	}); err != nil {
		plugin.Logger(ctx).Trace("gcp_storage_object.listStorageObjects", "api_error", err)
		return nil, err
	}

	return nil, err
}
  1. Initialize the SDK and authenticate with the necessary credentials.
  2. Make an initial API request with pagination enabled, specifying a page token.
  3. Retrieve the page token from the response and attempt to use it in subsequent API requests.
  4. Inspect the generated URL for the subsequent request.

While working with pagination using the page token, I noticed that the generated URL for subsequent API requests contains "MISSING" appended to the page token instead of the actual page token value

pageToken=Ckhsb2dzL2RhZ19pZD1haXJmbG93X21vbml0b3JpbmcvcnVuX2lkPW1hbnVhbF9fMjAyMy0wMi0yMlQxOTo1NDoyMiswMDowMC8%!!(MISSING)D(MISSING)

Error: Get "https://storage.googleapis.com/storage/v1/b/australia-southeast1-gdp-de-c6d33507-bucket/o?alt=json&maxResults=1000&pageToken=Ckhsb2dzL2RhZ19pZD1haXJmbG93X21vbml0b3JpbmcvcnVuX2lkPW1hbnVhbF9fMjAyMy0wMi0yMlQxOTo1NDoyMiswMDowMC8%!!(MISSING)D(MISSING)&prettyPrint=false&projection=full": context canceled
	Get "https://storage.googleapis.com/storage/v1/b/australia-southeast1-gdp-de-c6d33507-bucket/o?alt=json&maxResults=1000&pageToken=CmJsb2dzL2RhZ19pZD1haXJmbG93X21vbml0b3JpbmcvcnVuX2lkPW1hbnVhbF9fMjAyMy0wMi0xMlQyMjowOTozNiswMDowMC90YXNrX2lkPWVjaG8vYXR0ZW1wdD0xLmxvZw%!!(MISSING)D(MISSING)%!!(MISSING)D(MISSING)&prettyPrint=false&projection=full": context canceled

Expected behavior

The URL for subsequent API requests should contain the actual page token value and not have string "MISSING" in it

karanpopat avatar Jul 07 '23 06:07 karanpopat

Hey I will take a look at this, cc @tritone as an fyi.

@karanpopat can I ask why you aren't using the Storage client at cloud.google.com/go/storage ? It provides many more features and has dedicated engineering support. Not suggesting you should lift your entire app onto it, but just an fyi. THis package is actually deprecated: https://github.com/googleapis/google-api-go-client/blob/e871335ad6700d89d2b9629db99d1e674d5b9cad/storage/v1/storage-gen.go#L9

noahdietz avatar Jul 07 '23 16:07 noahdietz

@karanpopat It would help if you could make a repro of this that is runnable as a stand-alone. Just from looking at this, I see you are setting page.NextPageToken = "" which to me seems suspect; you should never have to set the page token to the empty string (the API will itself return an empty nextPageToken when there are no more pages to request).

tritone avatar Jul 07 '23 18:07 tritone

@tritone The check if d.RowsRemaining(ctx) == 0 is the condition that allows us to exit the loop and end our execution. Therefore, even if we set page.NextPageToken = "" at that point, ideally it should not affect the overall function of the call.

karanpopat avatar Jul 12 '23 08:07 karanpopat

@noahdietz I'll take a look at the package to see if I encounter any similar problems. However, I can't guarantee that we will be migrating to it at this stage.

karanpopat avatar Jul 12 '23 08:07 karanpopat

@karanpopat if you are trying to end execution then it looks like you need to return a non-nil error from Pages. See https://pkg.go.dev/google.golang.org/[email protected]/storage/v1#ObjectsListCall.Pages . Just returning a nil error means that execution continues.

tritone avatar Jul 12 '23 13:07 tritone

Closing as as by design. I don't think there is a bug to fix here, but please re-open if I am mistaken

codyoss avatar Apr 30 '24 21:04 codyoss