On 11/12/19 10:46 AM, Fabian Grünbichler wrote:
> On October 17, 2019 5:33 pm, Thomas Lamprecht wrote:
>> On 10/17/19 3:14 PM, Fabian Grünbichler wrote:
>>> @@ -1232,7 +1232,10 @@ sub unshift_read_header {
>>> } elsif ($path =~ m/^\Q$base_uri\E/) {
>>> my $token = $r->header('CSRFPreventionToken');
>>> my $cookie = $r->header('Cookie');
>>> - my $ticket =
>>> PVE::APIServer::Formatter::extract_auth_cookie($cookie,
>>> $self->{cookie_name});
>>> + my $auth_header = $r->header('Authorization');
>>> + my $ticket =
>>> PVE::APIServer::Formatter::extract_auth_value($cookie,
>>> $self->{cookie_name});
>>> + $ticket =
>>> PVE::APIServer::Formatter::extract_auth_value($auth_header,
>>> $self->{cookie_name})
>>> + if !$ticket;
>>
>> can we do this a bit more separate like:
>>
>> if (!$ticket && (my $auth_header = $r->header('Authorization')) {
>> $ticket = PVE::APIServer::Formatter::extract_auth_value($auth_header,
>> $self->{cookie_name});
>> }
>
> this would then (with the next patch) become:
>
> my $api_token;
> if (!$ticket && (my $auth_header = $r->header('Authorization')) {
> $ticket = PVE::APIServer::Formatter::extract_auth_value($auth_header,
> $self->{cookie_name});
>
> if (!$ticket) {
> $api_token =
> PVE::APIServer::Formatter::extract_auth_value($auth_header,
> $self->{apitoken_name});
> }
> }
>
the inner (apitoken) "if" would be nicer to move a layer out below the other
one.>>
>> or if you prefer:
>>
>> if (!$ticket) {
>> my $auth_header = $r->header('Authorization');
>> $ticket = PVE::APIServer::Formatter::extract_auth_value($auth_header,
>> $self->{cookie_name});
>> }
>
> my $api_token;
> if (!$ticket) {
> my $auth_header = $r->header('Authorization');
> $ticket = PVE::APIServer::Formatter::extract_auth_value($auth_header,
> $self->{cookie_name});
>
> if (!$ticket) {
> $api_token =
> PVE::APIServer::Formatter::extract_auth_value($auth_header,
> $self->{apitoken_name});
> }
> }
>
>>
>> makes it slightly cleaner/easier to read, IMO
>
> which makes it harder to parse IMHO, but if you still prefer it I will
> rewrite it with your suggestion for v2? we could of course also add
> comments, like so:
>
> # check actual cookie
s/check/prefer/
> my $ticket = PVE::APIServer::Formatter::extract_auth_value($cookie,
> $self->{cookie_name});
>
> # fallback to cookie in 'Authorization' header
> $ticket = PVE::APIServer::Formatter::extract_auth_value($auth_header,
> $self->{cookie_name})
> if !$ticket;
>
> # fallback to API token
> my $api_token = PVE::APIServer::Formatter::extract_auth_value($auth_header,
> $self->{apitoken_name})
> if !$ticket;
NAK, do *not* declare variables guarded with a postfix if, that gets
bad results.[0][1]
[0]: https://pve.proxmox.com/pipermail/pve-devel/2018-June/032238.html
[1]: https://perldoc.perl.org/perlsyn.html#Statement-Modifiers
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
