Support punycode bucket/alias mapping in the webserver #273

New Issue

Open

opened 2022-03-19 19:15:15 +00:00 by rune · 3 comments

rune commented

2022-03-19 19:15:15 +00:00

Contributor

Would it make sense to support punycode in the webserver? For example I have a website that uses the Danish letter ø, which e.g. nginx and haproxy support as long as I convert it to punycode everywhere.

It's directly against the spec for bucket names, but perhaps it could be supported in aliases?
https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html

Bucket names must not start with the prefix xn--.

The obvious workaround is that I have to re-configure my haproxy to rewrite the domain name, but it's not as clean.

Would it make sense to support punycode in the webserver? For example I have a website that uses the Danish letter `ø`, which e.g. nginx and haproxy support as long as I convert it to punycode everywhere. It's directly against the spec for bucket names, but perhaps it could be supported in aliases? https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html > Bucket names must not start with the prefix xn--. The obvious workaround is that I have to re-configure my haproxy to rewrite the domain name, but it's not as clean.

quentin added the

Improvement

label 2022-03-20 22:09:12 +00:00

quentin commented

2022-03-21 09:35:59 +00:00

Owner

The validation logic of the bucket name is here in the code: https://git.deuxfleurs.fr/Deuxfleurs/garage/src/branch/main/src/model/bucket_alias_table.rs#L66-L90

Earlier, in the projet design, we discussed the possibility of diverging from the S3 spec by using some flags. We might want to add some configuration flags to disable parts of the bucket name validation process.

eg.

[s3_api]
# ...
s3_extension_allow_punnycode_in_bucket_name = true # with a default of false

The validation logic of the bucket name is here in the code: https://git.deuxfleurs.fr/Deuxfleurs/garage/src/branch/main/src/model/bucket_alias_table.rs#L66-L90 Earlier, in the projet design, we discussed the possibility of diverging from the S3 spec by using some flags. We might want to add some configuration flags to disable parts of the bucket name validation process. eg. ```toml [s3_api] # ... s3_extension_allow_punnycode_in_bucket_name = true # with a default of false ```

rune commented

2022-03-21 15:12:00 +00:00

Author

Contributor

Hmm, I thought this would be a simple fix, but since is_valid_bucket_name is a public function in the root it can't access variables in BucketAliasTable, which I was going to instantiate with a copy of the s3_api config.

It seems like the only way, to get the config values into the function, is to update the signature and pass the problem up to the caller. Which means updating ~6 uses. A few of those don't have access to the config either, pushing the problem further up.

Maybe there's a better way?

Hmm, I thought this would be a simple fix, but since `is_valid_bucket_name` is a public function in the root it can't access variables in `BucketAliasTable`, which I was going to instantiate with a copy of the s3_api config. It seems like the only way, to get the config values into the function, is to update the signature and pass the problem up to the caller. Which means updating ~6 uses. A few of those don't have access to the config either, pushing the problem further up. Maybe there's a better way?

quentin referenced this issue

2022-04-11 09:19:59 +00:00

Make Garage's config global (lazy static) #292

lx commented

2022-04-12 09:57:13 +00:00

Owner

Basically we have two options to make this happen:

pass the relevant config variables all the way down to is_valid_bucket_name, which means passing this as a parameter to many intermediate functions, which is quite cumbersome
put Garage's configuration, or part of it, in a global variable

I think solution 2 is probably what we want, see details in #292.

Basically we have two options to make this happen: 1. pass the relevant config variables all the way down to `is_valid_bucket_name`, which means passing this as a parameter to many intermediate functions, which is quite cumbersome 2. put Garage's configuration, or part of it, in a global variable I think solution 2 is probably what we want, see details in #292.

quentin self-assigned this 2023-06-15 07:38:02 +00:00