S3 - Fix exceptions for VersionID (#3884)

[amarjandu]

Add inspection to ensure VersionID is a valid uuid v4
Add inspection to raise exception NoSuchVersion if key and version are missing

[amarjandu]

Having a difficult time getting test suite to run correctly... attempted to follow contributing guidelines but it appears that some unrelated tests are failing locally. Could I get some help from a maintainer?

[bblommers]

Hi @amarjandu, thanks for the PR! As far as I know, the version is not guaranteed to be in UUID format. Versions from a personal S3 bucket look like randomly generated alphanumeric strings, similar to the one used in your example.

I haven't been able to find any documentation on the specifics of what does determine a valid VersionId, so I'd suggest we do not have any checks on validity at all.

The current behaviour is definitely wrong though, so a fix for the current error message would be welcome :)

[amarjandu]

@bblommers I'm a little confused about the statement

As far as I know, the version is not guaranteed to be in UUID format

I assumed because there is limited detail about the versionID from the s3 api, Moto fell back to use uuid4s to to denote unique versions of an object. Is this assumption incorrect?

If moto is using uuid4s for versionIDs, I would think that is would be safe to inspect if a uuid4 is being used when the user requests for an object.

I suppose an alternative PR would be changing the versionID that moto uses from a uuid4 to an encoded string that has a time stamp value. Upon requests to moto the encoded string could be decoded and inspected for validity.

Yeah in the interim we should fix the error message, but we should also take a look at getting closer to support the s3 api.

Is the a committee of maintainers that large changes have to get approved by? Or do you just have to know one or two of the bouncers to get into the club? 😛

[bblommers]

Moto fell back to use uuid4s to to denote unique versions of an object

That assumption is correct, but if we use this as the basis for validation, the behaviour would be different between AWS and Moto.
I.e., the following would pass in Moto, but fail in AWS:

v_id = "71f5ebb0-697e-490f-871f-38cb834a66ec"
s3 = boto3.client("s3", region_name="us-east-1")
s3.get_object(Bucket=name, Key=key, VersionId=v_id)

The ideal situation/fix would be for Moto to use exactly the same format as AWS. Without knowing exactly what that is, however, I prefer Moto to be feature incomplete, rather than inconsistent.

Is the a committee of maintainers that large changes have to get approved by?

Yeah, big or far-reaching changes would have to be approved by spulec as well. I hope this PR will not become that big though :)

[bblommers]

https://docs.aws.amazon.com/AmazonS3/latest/userguide/versioning-workflows.html

Version IDs are Unicode, UTF-8 encoded, URL-ready, opaque strings that are no more than 1,024 bytes long.

Doesn't tell much about the specific format. Doing some simple testing, it almost looks like it verifies the input against a list of all known version ID's.
If the version ID is not known: Invalid version.
If the version ID is known, but does not belong to this key: Unknown key.

[amarjandu]

@bblommers I'm going to close this PR and open up a new one to address the exception message. We might want to keep #3884 open so we can track the migrating from a uuid4 to another format for the VersionID.

[amarjandu]

Raised #3887