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

storage: Incorrect page token value in the URL #2057

Closed
karanpopat opened this issue Jul 7, 2023 · 6 comments
Closed

storage: Incorrect page token value in the URL #2057

karanpopat opened this issue Jul 7, 2023 · 6 comments
Assignees
Labels
api: storage Issues related to the Cloud Storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@karanpopat
Copy link

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 karanpopat added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Jul 7, 2023
@product-auto-label product-auto-label bot added the api: storage Issues related to the Cloud Storage API. label Jul 7, 2023
@codyoss codyoss assigned noahdietz and unassigned codyoss Jul 7, 2023
@noahdietz
Copy link
Contributor

noahdietz commented Jul 7, 2023

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:

// This package is DEPRECATED. Use package cloud.google.com/go/storage instead.

@tritone
Copy link
Contributor

tritone commented Jul 7, 2023

@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).

@karanpopat
Copy link
Author

@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
Copy link
Author

@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.

@tritone
Copy link
Contributor

tritone commented Jul 12, 2023

@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/api@v0.130.0/storage/v1#ObjectsListCall.Pages . Just returning a nil error means that execution continues.

@codyoss
Copy link
Member

codyoss commented Apr 30, 2024

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 codyoss closed this as not planned Won't fix, can't repro, duplicate, stale Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

4 participants