usebruno / bruno

Opensource IDE For Exploring and Testing Api's (lightweight alternative to postman/insomnia)
https://www.usebruno.com/
MIT License
27.33k stars 1.25k forks source link

Bruno is altering response headers to lowercase when they shouldn't be #2012

Open DanaEpp opened 7 months ago

DanaEpp commented 7 months ago

Issue

It appears that Bruno lowercases header names, even if the server is not sending it back that way. This could potentially break tests that are relying on testing for a header by name where the case does not match.

Steps to reproduce issue

Attempt a curl to a server and look at the headers. In my case, I will be looking for the 'Server' header on crapi.apisec.ai.

image

Notice how "Server" starts with a capital S.

Now do the same request in Bruno.

image

Notice how everything is now lowercase.

Expected behavior

The case should be preserved so existing tests that are looking for the case can continue to pass.

DanaEpp commented 7 months ago

After a discussion on Discord with @Its-treason, I've come to accept that the header names should be lowercase since HTTP headers are supposed to be case-insensitive anyway. In fact, this makes it easier on the tests, since you never have to try to do a .toLowerCase() on a potentially undefined header.

I think this SHOULD be documented clearly though that the headers will always be lowercase when calling res.getHeader() so it is properly expected, as that is NOT the norm for most API clients like Postman.

Harti commented 7 months ago

I would like to point out that use-cases outside of RFCs do exist: Quickly testing/debugging legacy application APIs that are not RFC compliant.

Do you suggest I should use another software to preserve the original casing sent by the server? I would rather not leave Bruno behind.

mjhcorporate commented 7 months ago

@Harti : I agree with you. In this case, and in other cases (json-bodies!) Bruno should send requests exactly as I defined them. I need Bruno as a test tool, not as a generic, standards-compliant http client. Being able to intentionally break standards is a core requirement for me.

andreassiegel commented 6 months ago

Services like Azure API Management behave the same which bothered me in the beginning, too. By now, I actually like that the RFC is followed.

@mjhcorporate I understand you want to use case-sensitive headers in requests, and @Harti, you basically want to receive case-sensitive headers in responses.

Assuming some API is ignoring the RFC and relies on case-sensitive headers I see the point of sending requests as they are defined, but I wonder where case-sensitive headers in responses received by Bruno matter. This use case isn't that clear to me.

Harti commented 6 months ago

@andreassiegel The response header casing matters only for manual debugging purposes. I maintain a legacy server application that shares mixed-case headers with the corresponding proprietary client application. It relies on case-sensitivity due to bad (or lack of) design decisions made in the client accessing these headers. The client malfunctions if the casing isn't exactly as expected and that cannot be changed.

With every server/proxy/technology/framework/configuration update, I'm basically sweating and frantically double checking if the header casing is still as messed up as designed. Bruno however will not let me do this and instead gives me heart attacks when the headers suddenly are lowercased. 😉

It'd be awesome to have an option to inspect the response headers in their original state.

Does that make sense as a use-case?

andreassiegel commented 6 months ago

@Harti It certainly does! I did not think about using Bruno to check and ensure clients will get what they rely on. 🙈

Personally, I stopped using tools like Postman, etc. for testing already a while ago because I didn't want to maintain different technologies for unit test, integration tests and end-to-end tests. In my case, such tests are done using REST Assured.

Unfortunately, the header casing is not something Bruno decided to do. This is already done by axios, and they change the headers to lowercase intentionally: https://github.com/axios/axios/issues/413 Thus, "fixing" this behavior in Bruno meant replacing axios. 😬

pietrygamat commented 6 months ago

Replacing axios is not out of the question. Maybe you'll have some contribution to offer here: #2165?

Its-treason commented 6 months ago

Replacing Axios is definitely possible. I've rewritten the whole request logic to use node:http on my fork: https://github.com/Its-treason/bruno/blob/tmp-request-rewrite/packages%2Fbruno-common%2Fsrc%2Frequest%2Findex.ts Mostly so I can better inspect Headers / Redirect etc.

andreassiegel commented 6 months ago

I wonder if the underlying library for requests should be a configuration option so that the user can decide what to use, or whether that should rather be a conscious decision made by Bruno developers. 🤔

The issue https://github.com/usebruno/bruno/issues/2165 mentioned by @pietrygamat sounds like it should be configurable -- but why? As a user, I most likely don't care how a tool internally does its job (as long as it does it as expected).

mjhcorporate commented 6 months ago

+1 for replacing axios. However, I am against a toggle -- it makes no sense to maintain two backends, and also hinders support when for each issue we have to ask: "Which backend did you use"?

I would propose to migrate away from axios so that Bruno can send/receive requests exactly as they are given by the user / the server. Further, for standards-compliance, I would propose some mechanism that inspects Bruno collections for standards-compliance (a cli tool that can be used in a pre-commit hook?) and possibly a "strict" mode, where Bruno automatically asserts that responses are standard-compliant (i.e., if the server says he returns json but instead returns broken json, bruno would show an error).

tho-gru-38 commented 3 weeks ago

To my naive understanding Bruno access the header fields through res.getHeader("location").

What is the problem to do the .toLowerCase() call in the implementation of the getHeader method. In this case the result of res.getHeader("location") will be the same as res.getHeader("Location") (with uppercase L) and res.getHeader("LOCATION").