search

loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.95k stars 1.07k forks source link

"Content-Type" header is overwritten #5168

Closed agnes512 closed 3 years ago

agnes512 commented 4 years ago

@SephReed commented on Mon Apr 20 2020

Steps to reproduce

export class TestController {
  @get('/hello-page', {
    description: "Static page",
    responses: {}
  })
  async getHelloPage(
    @inject(RestBindings.Http.RESPONSE) res: Response,
  ) {
    res.setHeader("Content-Type", "text/html; charset=UTF-8");
    res.setHeader("Content-Type-Test", "text/html; charset=UTF-8");
    return `<html>Hello World</html>`;
  }
}

Current Behavior

Content type is text/plain

Screen Shot 2020-04-20 at 2 59 27 PM

A Content-Type-Test header was also set, it is the correct text/html; charset=UTF-8

Expected Behavior

Content-Type should be text/html; charset=UTF-8

jannyHou commented 4 years ago

@agnes512 it's probably caused by the default content type set in the write action: https://github.com/strongloop/loopback-next/blob/a81ce7e1193f7408d30d984d0c3ddcec74f7c316/packages/rest/src/writer.ts#L58

It happens after the controller function invoke, see https://github.com/strongloop/loopback-next/blob/a81ce7e1193f7408d30d984d0c3ddcec74f7c316/packages/rest/src/sequence.ts#L100-L103

bajtos commented 4 years ago

I think the current behavior is intentional. If the user wants to manipulate the response directly via @inject(RestBindings.Http.RESPONSE) then they should also send the response body using res.send.

For longer term, we should allow controller methods to return an object describing response status code and headers in addition to response body. See https://github.com/strongloop/loopback-next/issues/436

SephReed commented 4 years ago

Hmmm... okay. I've thought of modifying my sequence.ts to catch these cases and handle response.send() on it's own.

The issue is that circumnavigating response.send() seems like it's going to create issues down the line. I'm not sure what that function does, but assume it exists for a reason.

export class MySequence implements SequenceHandler {

  async handle(context: RequestContext) {
    const { request, response } = context;
    // do stuff
    const result = await magic();
    if (response.getHeader("Content-Type")) {
      response.send(result);
      return;
    }
    this.send(response, result); 
  }
}

EDIT: tested it out, and seems to be happy enough

stale[bot] commented 3 years ago

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

stale[bot] commented 3 years ago

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.