From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 499541FF16F
for <inbox@lore.proxmox.com>; Thu, 16 Jan 2025 08:32:36 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id C24D41983C;
Thu, 16 Jan 2025 08:32:32 +0100 (CET)
Message-ID: <f838efc6-fcbc-41b7-b391-8d5c49b32d9f@proxmox.com>
Date: Thu, 16 Jan 2025 08:31:59 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird Beta
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20250108084558.390324-1-d.csapak@proxmox.com>
<20250108084558.390324-2-d.csapak@proxmox.com>
<7ffc1b0e-1d46-4f27-a0a9-f523b32d3108@proxmox.com>
Content-Language: en-US
From: Dominik Csapak <d.csapak@proxmox.com>
In-Reply-To: <7ffc1b0e-1d46-4f27-a0a9-f523b32d3108@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.016 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [RFC PATCH http-server 1/2] add error message into
http body
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
On 1/15/25 17:08, Thomas Lamprecht wrote:
> Am 08.01.25 um 09:45 schrieb Dominik Csapak:
>> In our rust client, we can't access the http reason phrases[0], so let's
>> put them into the body itself if we don't specify an explicit content.
>>
>> our proxmox-client code in rust already uses the body as message if
>> there is one [1], so we get that automatically.
>>
>> 0: https://github.com/hyperium/http/issues/737
>> 1: https://git.proxmox.com/?p=proxmox.git;a=blob;f=proxmox-client/src/client.rs;h=9b078a9820405b22ca54c17ea4da4c586e0649b4;hb=refs/heads/master#l237
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> src/PVE/APIServer/AnyEvent.pm | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
>> index 24209a1..bd76488 100644
>> --- a/src/PVE/APIServer/AnyEvent.pm
>> +++ b/src/PVE/APIServer/AnyEvent.pm
>> @@ -388,6 +388,7 @@ sub error {
>> my ($self, $reqstate, $code, $msg, $hdr, $content) = @_;
>>
>> eval {
>> + $content //= $msg; # write error into body by default
>
> might this need altering the content-type or is that already to an
> OK default for a plain string? Just not that it's set to, e.g.,
> application/json but contains the error that cannot be parsed
> as JSON.
>
> I can look myself, but I thought you might have already done so when
> developing this.
>
> If that's fine I'd have nothing against applying this now.
The newly created response here is not setting an explicit content-type, and
according to the relevant part of the http spec[0], the client may assume
'application/octet-stream' or may try to identify the content-type
from the content itself.
While I guess most of the time we put strings in the error, it may happen
that this is something different here (since we sometimes simply
pass through the message from 'die') so explicitly setting
to text/plain could also be sometimes wrong here.
So I'd either leave it like this, or reformat the message to be e.g.
always text in some form, e.g. with
if (!defined($content)) {
$content = "Some error occured: $msg";
# set content type here
}
0: https://www.rfc-editor.org/rfc/rfc9110.html#name-content-type
>
>> my $resp = HTTP::Response->new($code, $msg, $hdr, $content);
>> $self->response($reqstate, $resp);
>> };
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel