Comments

[wip] Added tempUrl generating capability#251

Open

haphan wants to merge 2 commits intomaster from

swift-tempurl

Open

[wip] Added tempUrl generating capability #251
haphan wants to merge 2 commits intomaster from
swift-tempurl

Conversation

@haphan

Copy link

Collaborator

@haphan haphan commented Aug 22, 2018 •

edited

Loading

$swiftService = $openstack->objectStoreV1();
$expire = time() + 3600;
$path = '/v1/AUTH_account/container/object';
$key = 'mys33rtkey';
$tempUrl = $swiftService->tempUrl('GET', $expire, $path, $key);

This method loosely follows swift command's way to generate temporary url:

swift tempurl $METHOD $EXPIRE $PATH $KEY

Some features this PR tries to support:

tempUrl for single object
tempUrl for multiple objects by prefix-based path
multiple hash digest algorithm (sha1, sha256, sha512)
ip range restriction

@haphan


 Added tempUrl generating capability

@haphan haphan force-pushed the swift-tempurl branch from 67c32b2 to 6983012 Compare

August 29, 2018 03:31

@haphan haphan mentioned this pull request

Aug 29, 2018

Added update method to ObjectStorage #250

Closed

@haphan haphan changed the title ~~(削除) Added tempUrl generating capability (削除ここまで)~~ (追記) [wip] Added tempUrl generating capability (追記ここまで)

Aug 29, 2018

MSC29

MSC29 suggested changes

Feb 14, 2020

View reviewed changes

Copy link

@MSC29 MSC29 left a comment

Choose a reason for hiding this comment

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

Recently used this code & stumbled upon a issue when using container objects with spaces in their name...
The hash used the non-encoded path but the temporary url queried agaisnt openstack needs to be url encoded.

src/ObjectStore/v1/Service.php

* @param string $method An HTTP method to allow for this temporary URL. Any of GET, POST, HEAD, PUT, POST, DELETE.

* @param int $expires Unix timestamp

* @param string $path The full path or storage URL to the Swift object. Example: '/v1/AUTH_account/c/o' or: 'http://saio:8080/v1/AUTH_account/c/o'

Copy link

@MSC29 MSC29 Feb 14, 2020

Choose a reason for hiding this comment

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

Hi there. I just re-used this code as I need to generate temporary urls.
It took me a while to notice the issue but here it is in the doc "Do not URL-encode the path when you generate the HMAC-SHA1 signature. However, when you make the actual HTTP request, you should properly URL-encode the URL.".

I'd add this to the doc to ensure an non-encoded $path is passed in.

src/ObjectStore/v1/Service.php

// sha512 requires prefixing signature

$signature = 'sha512' === $digest ? 'sha512:'.$signature : $signature;

return sprintf('%s?temp_url_sig=%s&temp_url_expires=%s', $path, $signature, $expires);

Copy link

@MSC29 MSC29 Feb 14, 2020

Choose a reason for hiding this comment

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

Given the $path is not encoded, I'd suggest encoding the actual temporary url path

Suggested change

return sprintf('%s?temp_url_sig=%s&temp_url_expires=%s', $path, $signature, $expires);

return sprintf('%s?temp_url_sig=%s&temp_url_expires=%s', urlencode($path), $signature, $expires);

@drzraf

Copy link

drzraf commented May 11, 2020 •

edited

Loading

The function returns the signature (what is needed from this function) bundled with $expires and $path (callers just passed as arguments).

Imagine you have a Psr\Http\Message\UriInterface (as obtained by by$container->getObject('')->getPublicUri();)

You rather only want the signature, in order to call:
$uri->withQuery(sprintf('temp_url_sig=%s&temp_url_expires=%s', $signature, $expires))->__toString();

It's especially true if a prefix is used because the prefix:<path> format expected for hash-generation is not a path suitable for actual Uri building.
I think the function should just return the $signature (string)

And urlencode() would be up to the caller.

@haphan


 Merge branch 'master' into swift-tempurl

695a2b4

Labels

None yet

3 participants

@haphan @drzraf @MSC29

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

[wip] Added tempUrl generating capability#251

[wip] Added tempUrl generating capability #251
haphan wants to merge 2 commits intomaster from
swift-tempurl

Conversation

@haphan haphan commented Aug 22, 2018 •

edited

Loading

Uh oh!

Uh oh!

@MSC29 MSC29 left a comment

Choose a reason for hiding this comment

Uh oh!

@MSC29 MSC29 Feb 14, 2020

Choose a reason for hiding this comment

Uh oh!

@MSC29 MSC29 Feb 14, 2020

Choose a reason for hiding this comment

Uh oh!

drzraf commented May 11, 2020 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

@haphan haphan commented Aug 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

@MSC29 MSC29 left a comment

Choose a reason for hiding this comment

Uh oh!

@MSC29 MSC29 Feb 14, 2020

Choose a reason for hiding this comment

Uh oh!

@MSC29 MSC29 Feb 14, 2020

Choose a reason for hiding this comment

Uh oh!

drzraf commented May 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

@haphan haphan commented Aug 22, 2018 •

edited

Loading

drzraf commented May 11, 2020 •

edited

Loading