It would be great to have a better commit message, or better said:
one at all. You cover-letter looks like it could be a good fit for
that here, almost as-is.
On 8/30/19 2:12 PM, Tim Marx wrote:
> Signed-off-by: Tim Marx <t.m...@proxmox.com>
> ---
> PVE/APIServer/AnyEvent.pm | 5 +++++
> PVE/APIServer/Formatter.pm | 12 ++++++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/PVE/APIServer/AnyEvent.pm b/PVE/APIServer/AnyEvent.pm
> index 2e8ca47..c8f7b6d 100644
> --- a/PVE/APIServer/AnyEvent.pm
> +++ b/PVE/APIServer/AnyEvent.pm
> @@ -1223,6 +1223,11 @@ sub unshift_read_header {
> my $cookie = $r->header('Cookie');
> my $ticket =
> PVE::APIServer::Formatter::extract_auth_cookie($cookie, $self->{cookie_name});
>
> + if (!defined $ticket) {
please use parenthesis with defined checks, e.g.:
if (!defined($ticket)) {
You could also do:
if (!defined($ticket) && (my $authHeader = $r->header('Authorization'))) {
Here, but a bit longer / more nested, so just a suggestion.
Not sure about the change from "=" to space " " as a separation.
"Authorization" is specified in detail by any ietf or similar RFC,
but it seems that it has some basic rules set in place [0].
[0]: https://tools.ietf.org/html/rfc7235#section-4.2
So with we being the server, and knowing that proxies may not alter
this field we could just use the same format as the ticket and reuse
the method. Albeit, I'd rename it then to "extract_auth_ticket" or
the like.
If you find anything that specifies reasons for why a space separation
would be better here, or if you just insist on it, we could also extend
the regex to allow either " " or "=" as separator and still reuse the
same (renamed) "extract_auth_ticket" method. Something like:
my $ticket = ($auth_header =~ /(?:^|\s)\Q$type\E(?:[ =])([^;]*)/)[0];
should do the trick (did not tested it).
Rest looks OK for me.
> + my $authHeader = $r->header('Authorization');
> + $ticket =
> PVE::APIServer::Formatter::extract_ticket_from_auth_header($authHeader,
> $self->{cookie_name});
> + }
> +
> my ($rel_uri, $format) = &$split_abs_uri($path,
> $self->{base_uri});
> if (!$format) {
> $self->error($reqstate, HTTP_NOT_IMPLEMENTED, "no such
> uri");
> diff --git a/PVE/APIServer/Formatter.pm b/PVE/APIServer/Formatter.pm
> index 0c459bd..f626180 100644
> --- a/PVE/APIServer/Formatter.pm
> +++ b/PVE/APIServer/Formatter.pm
> @@ -87,6 +87,18 @@ sub extract_auth_cookie {
> return $ticket;
> }
>
> +sub extract_ticket_from_auth_header {
> + my ($auth_header, $type) = @_;
> +
> + return undef if !$auth_header;
> +
> + my $ticket = ($auth_header =~ /(?:^|\s)\Q$type\E ([^;]*)/)[0];
> +
> + $ticket = uri_unescape($ticket) if $ticket;
> +
> + return $ticket;
> +}
> +
> sub create_auth_cookie {
> my ($ticket, $cookie_name) = @_;
>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
