apostrophe icon indicating copy to clipboard operation
apostrophe copied to clipboard

3.x: asset paths are not respecting baseUrl

Open myovchev opened this issue 2 years ago • 12 comments

To Reproduce

UPDATE: Actually it is more complex than that because of the apos.prefix config option. See my first comment with the additional findings.

Add pathname to the baseUrl config value in app.js:

// app.js
require('apostrophe')({
  baseUrl: 'http://somedomain.com/root',
  // ...
});

Assets are linked with paths /apos-frontend/releases/... while they should be /root/apos-frontend/releases/...

Expected behavior

Respect the configured baseUrl pathname.

Describe the bug

The whole issue is easily fixable via the dedicated method to get the base asset path. A quick fix would be just adding self.apos.baseUrl when appropriate. However, keeping the absolute path sounds like a great idea. This is my monkey patching (used on current staging apps) in order to illustrate it:

// modules/@apostrophecms/asset/index.js
const remote = process.env.APOS_UPLOADFS_ASSETS;

module.exports = {
  extendMethods(self) {
    return {
      getAssetBaseUrl(_super) {
        const url = _super();
        const baseUrl = self.apos.baseUrl;

        if (remote || !baseUrl) {
          return url;
        }

        const { pathname } = new URL(baseUrl);
        if (pathname !== '/') {
          return pathname + url;
        }

        return url;
      }
    };
  }
};

The above logic could be applied once on init and cached as property. And some additional try - catch security might be needed to ensure URL is not malformed.

myovchev avatar Nov 25 '21 11:11 myovchev

This seems to be not-so-simple issue based on my latest findings. There is also apos.prefix into the play which raises a discussion about how multiple Apostrophe apps are setup under a single domain. apos.prefix is used on the express level (router). I'm not sure if it has anything to do with e.g. multi-lingual prefix based link building.

Additionally Apostrophe uses req.url internally which effectively strips the apos.prefix (the difference between req.url and req.originalUrl is described in the express docs).

So the question would be how we setup an app to be delivered at https://somedomain.com/some-app given the fact we have the appropriate nginx mapping? Variant 1:

require('apostrophe')({
  baseUrl: 'http://somedomain.com',
  prefix: '/some-app'
  // ...
});

Variant 2:

require('apostrophe')({
  baseUrl: 'http://somedomain.com/some-app'
  // ...
});

In any case the above bug is still in place, just the solution may vary. Additionally in both cases pagination is broken (because of req.url), but this is a different issue which I may raise when I have a clear understanding about the mentioned configuration options.

myovchev avatar Nov 28 '21 12:11 myovchev

We haven't done a lot of validation of prefix support in A3 yet, so I'm not overly surprised that there are some loose ends with that.

Your first example would be correct re: baseUrl versus prefix, for consistency with how we did this in A2 (and how it almost works now in A3).

It sounds to me like we just need to sort out an issue with getAssetBaseUrl when prefix is present, although we can't do that all the time (it only makes sense when local storage is being used).

On Sun, Nov 28, 2021 at 7:25 AM Miro Yovchev @.***> wrote:

This seems to be not-so-simple issue based on my latest findings. There is also apos.prefix into the play which raises a discussion about how multiple Apostrophe apps are setup under a single domain. apos.prefix is used on the express level (router). I'm not sure if it has anything to do with e.g. multi-lingual prefix based link building.

Additionally Apostrophe uses req.url internally which effectively strips the apos.prefix (the difference between req.url and req.originalUrl is described in the express docs https://expressjs.com/en/api.html#req.originalUrl).

So the question would be how we setup an app to be delivered at https://somedomain.com/some-app given the fact we have the appropriate nginx mapping? Variant 1:

require('apostrophe')({ baseUrl: 'http://somedomain.com', prefix: '/some-app' // ...});

Variant 2:

require('apostrophe')({ baseUrl: 'http://somedomain.com/some-app' // ...});

In any case the above bug is still in place, just the solution may vary. Additionally in both cases pagination is broken (because of req.url), but this is a different issue which I may raise when I have a clear understanding about the mentioned configuration options.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/3559#issuecomment-981076693, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27N3WQNS75JSK64Y7ATUOINUHANCNFSM5IYLH4PA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

boutell avatar Nov 28 '21 20:11 boutell

It sounds exactly as the example I gave above (there is a remote check), except it shouldn't parse the baseUrl but use prefix if it's available. I'll patch my staging environment to be consistent with your direction and I'll fill another issue related with req.url/data.url issue.

myovchev avatar Dec 01 '21 08:12 myovchev

Hello. Is prefix not supported in 3.x? What would be the recommended approach for running on a subdirectory? Thank you

JuanIrache avatar Apr 07 '22 15:04 JuanIrache

It supports prefix configuration (see above), but you should watch out for req.url usage (e.g. pager macro). I'm also not sure if static asset URL's builder will pickup the prefix value, but you could patch it up from within your application - just extend getAssetBaseUrl as shown above, but only prepend the super() result with self.apos.prefix. The whole prefix thing just needs "fine tuning" in the near future.

myovchev avatar Apr 08 '22 12:04 myovchev

Prefix is supported. You should not put a path component in baseUrl, you should put that in prefix.

It's true we haven't done a lot of validation of the prefix option in A3 yet. If you do find any bug fixes to be necessary I would recommend sending a PR and not using a project-level workaround, as we will otherwise at some point make a bug fix that causes problems for your workaround.

boutell avatar Apr 08 '22 13:04 boutell

Is there any v3 documentation for how to use prefix and baseUrl?

I'm trying this out and running v3 with nginx in front, proxying /apos to apostrophe in a docker container. All links on the initial page are assuming it is at /, and if I manually enter /apos/login to navigate there, the resources it tries to fetch do not include the prefix in their paths. Login is a built-in page, and it doesn't seem to respect the prefix configuration.

Or prefix is configured differently than in v2? I could only find v2 docs for that.

This is in my app.js:

require('apostrophe')({
  shortName: 'experiment-blog',
  prefix: '/apos',
  modules: {
    // Apostrophe module configuration
    // *******************************
    //
    // NOTE: most configuration occurs in the respective modules' directories.
    // See modules/@apostrophecms/page/index.js for an example.
    //
    // Any modules that are not present by default in Apostrophe must at least
    // have a minimal configuration here to turn them on: `moduleName: {}`
    // ***********************************************************************
    // `className` options set custom CSS classes for Apostrophe core widgets.
    '@apostrophecms/rich-text-widget': {
      options: {
        className: 'bp-rich-text'
      }
    },
    '@apostrophecms/image-widget': {
      options: {
        className: 'bp-image-widget'
      }
    },
    '@apostrophecms/video-widget': {
      options: {
        className: 'bp-video-widget'
      }
    },
    // `asset` supports the project's webpack build for client-side assets.
    asset: {},
    // The project's first custom page type.
    'default-page': {}
  }
});

And my nginx config excerpt:

  location /apos {
    proxy_pass         http://apos_upstream;
    proxy_intercept_errors off;
    error_page 404 = /apos;
    proxy_set_header   Host $host;
    proxy_set_header   X-Real-IP $remote_addr;
    proxy_set_header   X-Forwarded-For $proxy_add_x_forwarded_for;
    proxy_set_header   X-Forwarded-Host $server_name;
    proxy_set_header Upgrade $http_upgrade;
    proxy_set_header Connection "Upgrade";
  }

Any v3 documentation links or confirmation that this either works or doesn't would be appreciated.

Should I use v2 instead if v3 isn't ready to do this? Is anyone successfully running apostrophe in a subdirectory in v3?

mnebuerquo avatar May 05 '22 20:05 mnebuerquo

Prefix is intended to be supported in A3, and there is some passing unit test coverage of this as well. However that coverage doesn't yet cover whether "_url" takes it into account, and it's possible that was missed, as we haven't had a use case of our own for this yet with an A3 customer. I suggest you take a look at test/express.js, especially this test:

  it('should successfully make a GET request to establish CSRF (prefix)', async function() {
    jar = apos.http.jar();
    const body = await apos.http.get('/prefix/tests/welcome', {
      jar
    });
    assert(body.toString() === 'ok');
  });

With a little more effort this could also cover whether data.page._url was emitted correctly in the test page template.

My instinct is that if it doesn't work already, it's a few lines of code from working correctly.

This is something we will likely have more demand for at some point in the near future and take care of ourselves. Or you could pursue the above notes and work toward a PR, or arrange for us to prioritize it as an enterprise customer, if you are not already (if interested in that option, please reach out to [email protected]).

boutell avatar May 05 '22 20:05 boutell

@boutell , thanks for your reply.

Unfortunately, I am not a customer at all yet, so I won't be able to influence your feature roadmap like an enterprise customer. I'm currently evaluating the technical feasibility of using apostrophe as a component of my client's architecture. I'm interested in the headless aspect especially, since the client has a react-native app and a website. I'm envisioning some content being edited in the cms and rendered on both platforms.

My thought was to use the built-in apostrophe page rendering for the blog and configure the layout.html to match the rest of the site, then later build some page rendering in react-native to show content in-app. The rest of the site is nextjs, data driven from its own API, so apostrophe would need to live in a subdirectory. I could put the admin login in a subdomain, but for apostrophe to render the blog pages it would need to be in the main domain. To make that work, I need prefix, because it calls for assets in various paths that already belong to other services.

The only other way to use this would be to go all-headless and run apostrophe in a subdomain just for the admin and api. If the page content is provided in a sensible way by the api (json, not html), then I can write my own renderer, which I was hoping to not need immediately.

I'm going to try installing v2, since the prefix is documented there, and it sounds like this is an issue with v3.

mnebuerquo avatar May 05 '22 20:05 mnebuerquo

Hi Sherman, that will certainly work, and perhaps later migration to A3 is the lesser task from your point of view. I do think we're rather close to full prefix support in A3 and it would make a good community contribution, but we all have to choose our battles!

On Thu, May 5, 2022 at 4:51 PM Sherman Adelson @.***> wrote:

@boutell https://github.com/boutell , thanks for your reply.

Unfortunately, I am not a customer at all yet, so I won't be able to influence your feature roadmap like an enterprise customer. I'm currently evaluating the technical feasibility of using apostrophe as a component of my client's architecture. I'm interested in the headless aspect especially, since the client has a react-native app and a website. I'm envisioning some content being edited in the cms and rendered on both platforms.

My thought was to use the built-in apostrophe page rendering for the blog and configure the layout.html to match the rest of the site, then later build some page rendering in react-native to show content in-app. The rest of the site is nextjs, data driven from its own API, so apostrophe would need to live in a subdirectory. I could put the admin login in a subdomain, but for apostrophe to render the blog pages it would need to be in the main domain. To make that work, I need prefix, because it calls for assets in various paths that already belong to other services.

The only other way to use this would be to go all-headless and run apostrophe in a subdomain just for the admin and api. If the page content is provided in a sensible way by the api (json, not html), then I can write my own renderer, which I was hoping to not need immediately.

I'm going to try installing v2, since the prefix is documented there, and it sounds like this is an issue with v3.

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/3559#issuecomment-1119035455, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27M7DAMGICVUVETLNALVIQYF5ANCNFSM5IYLH4PA . You are receiving this because you were mentioned.Message ID: @.***>

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

boutell avatar May 05 '22 20:05 boutell

@mnebuerquo There are minor issues related with the prefix as far as I remember. I have running in staging prefixed a3 apps. For example you need to modify the pagination examples (the core pager macro) in pieces index from the a3 docs - data.url should become data.absoluteUrl. You may also need to tweak some asset paths. Generally it's all templates related so not a big deal and shouldn't be a showstopper for your evaluation. I'll gladly assist you in solving those once you start implementing - just ping me in the Discussions here or in the Discord channel.

myovchev avatar May 06 '22 09:05 myovchev

Just keep in mind that we are likely to fix such issues in the core at some point, so when we do any workarounds could require adjustment.

On Fri, May 6, 2022 at 5:26 AM Miro Yovchev @.***> wrote:

@mnebuerquo https://github.com/mnebuerquo There are minor issues related with the prefix as far as I remember. I have running in staging prefixed a3 apps. For example you need to modify the pagination examples (the core pager macro) in pieces index from the a3 docs - data.url should become data.absoluteUrl. You may also need to tweak some asset paths. Generally it's all templates related so not a big deal and shouldn't be a showstopper for your evaluation. I'll gladly assist you in solving those once you start implementing - just ping me in the Discussions here or in the Discord channel.

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe/issues/3559#issuecomment-1119425325, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27KFQYS6FS3YGQXOOLDVITQTZANCNFSM5IYLH4PA . You are receiving this because you were mentioned.Message ID: @.***>

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

boutell avatar May 06 '22 12:05 boutell