Skip to content

Conversation

@drzraf
Copy link

@drzrafdrzraf commented Mar 27, 2024

I guess a Guzzle update or the swift provider HTTP server or a Swift update (or an intermediary proxy) changed the case of the HTTP headers and... getMetadata() started being empty what drove me crazy until I understood it.

@drzrafdrzraf changed the title HTTP headers key are case-insensitiveHTTP header names are case-insensitiveMar 27, 2024
…wercased name too so that code can consistently rely on it
Copy link
Member

@k0kak0ka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need a unit test for this case

$name = substr($header, strlen(static::METADATA_PREFIX));
$metadata[$name] = $response->getHeader($header)[0];
if ($name !== strtolower($name)){
$metadata[strtolower($name)] = $response->getHeader($header)[0];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also not sure we need this at all. It might break the backward compatibility as swift metadata would have double keys.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It allows my $container->getMetadata()["temp-url-key"] code to work indistinctly on both OVH swift 2.33 (and its all-lowercase headers) but also on another provider's Swift 2.25 with mixed-case headers' names.

@drzraf
Copy link
Author

need a unit test for this case

I'm sorry, I can't enter into such a thing.

@k0ka
Copy link
Member

k0ka commented Apr 2, 2024

I merged case insensitive comparison. But filling the metadata must be the same for backward compatibility. I can release "uniform" metadata in the next major release, but I'm not planning it in the near future.

You might create an interface to check metadata case-insensitive. Like $object->hasMetadataKey($key), $object->getMetadataKey($key) instead of manually checking keys of $object->getMetadata() array. But this will also require tests.

@k0kak0ka closed this Apr 2, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@drzraf@k0ka