Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
lnfel opened this issue Dec 19, 2023 · 13 comments
Labels
pending Pending for issue reporter to verify the fix

Comments

@lnfel
Copy link

lnfel commented Dec 19, 2023

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
Copy link

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
Copy link

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
Copy link

cholasimmons commented Jan 19, 2024

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
Copy link

Any one have figured out this particular issue?

@artemtam
Copy link

artemtam commented Feb 10, 2024

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
Copy link

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

@oSethoum
Copy link

same issue here this is really annoying.

@Hiutaky
Copy link

Hiutaky commented Jun 6, 2024

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
Copy link

binamralamsal commented Jul 7, 2024

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
Copy link
Member

SaltyAom commented Jul 9, 2024

Updating to 1.0.4 should fix the problem.

@SaltyAom SaltyAom added the pending Pending for issue reporter to verify the fix label Jul 9, 2024
@cannap
Copy link

cannap commented Jul 9, 2024

Updating to 1.0.4 should fix the problem.

works for me

@nick-cjyx9
Copy link

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
Copy link

Now I'm using a additional hook to solve this

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending Pending for issue reporter to verify the fix
Projects
None yet
Development

No branches or pull requests