elysiajs / elysia-cors

Plugin for Elysia that for Cross Origin Requests (CORs)
MIT License
31 stars 9 forks source link

[BUG] Access-Control-Allow-Origin returns empty string when origin is set to url string #41

Open lnfel opened 10 months ago

lnfel commented 10 months ago

Sample use case, considering we want to limit origin to a specific client app url. One would use cors plugin like the following:

const app = new Elysia()
  .use(cors({
    origin: 'http://localhost:5173'
  }))
  .get('/', () => {
    return 'Oh hi!'
  })

Expected to have Access-Control-Allow-Origin equal to the app url which is http://localhost:5173. But the cors plugin returns empty string instead: image

PureDizzi commented 10 months ago

I experienced this a while back and it was user error not not Elyisia. I don't remember what was the problem. Maybe credentials?

ChristianVaughn commented 9 months ago

Any solution to this? i tried setting a regex and i tried just a string with domainname.com and they both return the blank string

cholasimmons commented 9 months ago

Yea this has taken me 2 days to notice that the headers are actually being set to ''; Only when you set origin: true does it work, which ofcourse is not acceptable in production. So setting origin to a string or array of strings (of domain names) just sets the header to ''

--- EDIT ---

Just noticed from issue #5 One can also set a domain as /localhost.*/ ?? Neato, I would have never guessed that

abielzulio commented 9 months ago

Any one have figured out this particular issue?

artemtam commented 8 months ago

The workaround: remove protocol from the origin value:

const app = new Elysia()
  .use(cors({
    origin: 'localhost:5173' // this works, but allows all protocols
  }))
  .get('/', () => {
    return 'Oh hi!'
  })

Root cause: processOrigin method strips protocol from the request origin header and compares it to the origin from the plugin config. The plugin should either strip protocol from configured allowed origin or do not strip protocol from the request origin.

For reference, cors package does not strip anything and compares request origin to allowed origins as is. Also, by the spec, Origin is a protocol + hostname + port, I think the plugin shouldn't do any magic by stripping & ignoring protocols.

I would go with comparing request origin with allowed origin as is. @SaltyAom if you approve solution, I can raise a PR 🙏

abhi-slash-git commented 7 months ago

Very nuanced bug, either we update documentation with examples or fix it to follow other implementations.

oSethoum commented 6 months ago

same issue here this is really annoying.

Hiutaky commented 4 months ago

The workaround: remove protocol from the origin value:

const app = new Elysia()
  .use(cors({
    origin: 'localhost:5173' // this works, but allows all protocols
  }))
  .get('/', () => {
    return 'Oh hi!'
  })

Root cause: processOrigin method strips protocol from the request origin header and compares it to the origin from the plugin config. The plugin should either strip protocol from configured allowed origin or do not strip protocol from the request origin.

For reference, cors package does not strip anything and compares request origin to allowed origins as is. Also, by the spec, Origin is a protocol + hostname + port, I think the plugin shouldn't do any magic by stripping & ignoring protocols.

I would go with comparing request origin with allowed origin as is. @SaltyAom if you approve solution, I can raise a PR 🙏

Worked fine for me, really strange problem btw

binamralamsal commented 3 months ago

Looks like this issue is fixed in 1.0.3 version but it's not available in npm

I just copy pasted the file from the source code and used that instead and it works.

SaltyAom commented 3 months ago

Updating to 1.0.4 should fix the problem.

cannap commented 3 months ago

Updating to 1.0.4 should fix the problem.

works for me

nick-cjyx9 commented 2 months ago

Doesn't work again whether I add the protocol or not on cloudflare workers

https://github.com/nick-cjyx9/Random-Elysia/blob/f7a909a07ca4edfed62339a7f925429748fc8839/backend/src/controllers/all.ts#L15

image

version 1.1.0

nick-cjyx9 commented 2 months ago

Now I'm using a additional hook to solve this

app.onRequest(({ set }) => {
  set.headers['access-control-allow-credentials'] = 'true'
})