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

Can't not supply an expires in /sharing/create_shared_link_with_settings #75

Closed
ncw opened this issue Apr 6, 2021 · 1 comment · Fixed by #76
Closed

Can't not supply an expires in /sharing/create_shared_link_with_settings #75

ncw opened this issue Apr 6, 2021 · 1 comment · Fixed by #76
Labels

Comments

@ncw
Copy link
Contributor

ncw commented Apr 6, 2021

Describe the bug

I'd like to create a sharing link with supplying sharing.SharedLinkSettings but without supplying sharing.SharedLinkSettings.Expires

To Reproduce

Submit a request like this

	createArg := sharing.CreateSharedLinkWithSettingsArg{
		Path: absPath,
		Settings: &sharing.SharedLinkSettings{
			RequestedVisibility: &sharing.RequestedVisibility{
				dropbox.Tagged{sharing.RequestedVisibilityPublic},
			},
			Audience: &sharing.LinkAudience{
				dropbox.Tagged{sharing.LinkAudiencePublic},
			},
			Access: &sharing.RequestedLinkAccessLevel{
				dropbox.Tagged{sharing.RequestedLinkAccessLevelViewer},
			},
		},
	}
	var linkRes sharing.IsSharedLinkMetadata
	linkRes, err = f.sharing.CreateSharedLinkWithSettings(&createArg)

The actual JSON submitted to the endpoint is this (formatted for readability)

{
  "path": "/hello.txt",
  "settings": {
    "requested_visibility": {
      ".tag": "public"
    },
    "expires": "0001-01-01T00:00:00Z",
    "audience": {
      ".tag": "public"
    },
    "access": {
      ".tag": "viewer"
    }
  }
}

Note the "expires": "0001-01-01T00:00:00Z"

Expected Behavior

I would expect the expires field to be missing or possibly empty string

Actual Behavior

There may be a special case for this in the dropbox API, but expires is not supported by a non enterprise dropbox, so if you supply anything in the expires you get

{
  "error_summary": "settings_error/not_authorized/.",
  "error": {
    ".tag": "settings_error",
    "settings_error": {
      ".tag": "not_authorized"
    }
  }
}

Versions

This reproduces with the SDK at master

Additional context

Because time.Time is struct, its empty value is all the fields empty, hence the "0001-01-01T00:00:00Z".

A patch like this does fix the problem - making the time.Time into a pointer type so it can be omitted properly.

diff --git a/dropbox/sharing/types.go b/dropbox/sharing/types.go
index 085cda4..6ab9870 100644
--- a/dropbox/sharing/types.go
+++ b/dropbox/sharing/types.go
@@ -3922,7 +3922,7 @@ type SharedLinkSettings struct {
        LinkPassword string `json:"link_password,omitempty"`
        // Expires : Expiration time of the shared link. By default the link won't
        // expire.
-       Expires time.Time `json:"expires,omitempty"`
+       Expires *time.Time `json:"expires,omitempty"`
        // Audience : The new audience who can benefit from the access level
        // specified by the link's access level specified in the `link_access_level`
        // field of `LinkPermissions`. This is used in conjunction with team

However that needs to be done in the generator. It is also not backwards compatible.

An alternate patch would be to make a special case MarshalJSON for the Expires which checked expires = time.Time{} and omitted it.

@ncw ncw added the bug label Apr 6, 2021
ncw added a commit to rclone/rclone that referenced this issue Apr 6, 2021
@rajatgoel
Copy link
Contributor

So as long as we bump the major version when we make the next release, changing the type should be fine? If so, I think it should be fine to land #76.

ncw added a commit to rclone/rclone that referenced this issue Apr 7, 2021
negative0 pushed a commit to negative0/rclone that referenced this issue Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants