ProtonMail / WebClients

Monorepo hosting the proton web clients
GNU General Public License v3.0
4.24k stars 538 forks source link

API_URL not respected for "core/v4/images" and "core/v4/images/logo" API calls #311

Closed vladimiry closed 1 year ago

vladimiry commented 1 year ago

I'd expect all the remote API calls to go via API_URL defined in src/app/config.ts but for at least core/v4/images and core/v4/images/logo API calls the origin is being taken from window.location vs API_URL value.

Besides, according to https://github.com/ProtonMail/WebClients/issues/276#issuecomment-1138623698 the /api prefix should not be used, but recently it got explicitly added to the listed calls with api/ is required to set the AUTH cookie comment (see https://github.com/ProtonMail/WebClients/commit/1917e37f5d9941a3459ce4b0177e201e2d94a622#diff-77d6ccc0b7e167f474b04176de2ed0e90595175547e247cb59fe0d343b1cd616R114 and https://github.com/ProtonMail/WebClients/commit/7cc2cabb34c2e0426278d73af8356272f7322fc3#diff-e746f626e32ca02d5bd2db3f772f624d2c7f89f05fc23582b403d2b6d7516a08R61 for example).

I've put API_URL prefix for those calls, switched to <app-type>-api API subdomain use scenario vs /api prefix, and it works well.

CC @bartbutler

vladimiry commented 1 year ago

I guess the getApiSubdomainUrl function might be helpful for the need.

bartbutler commented 1 year ago

Huh, that's very strange. I don't know why it wouldn't use API_URL. @EpokK do you know?

vladimiry commented 1 year ago

I don't know why it wouldn't use API_URL

In those case, it doesn't do the API call via XHR wrapper/helper, but by setting relative api/core/v4/images / api/core/v4/images/logo src to img element and so origin gets automatically resolved from the loaded page (from the window.location).

bartbutler commented 1 year ago

We're looking at it, we think it should respect API_URL as well on first glance and will fix it if so.

vladimiry commented 1 year ago

Thanks. I replaced api with API_URL value in const prefixedUrl = `api/${config.url}` code part in forgeImageURL / getLogo functions, and it started working fine for my need.

EpokK commented 1 year ago

Thanks for the report @vladimiry, the hotfix will be present in Proton Mail 5.0.20.

vladimiry commented 1 year ago

Thanks for the update. Closing as resolved then.