Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#1186 closed defect (fixed)

strip xinha_config.baseHref AND location.href from paths

Reported by: ray Owned by: gogo
Priority: normal Milestone: 0.95
Component: Xinha Core Version: trunk
Severity: normal Keywords:
Cc:

Description

If a baseHref is set (e.g. on a different server) and xinha_config.stripBaseHref is active both the foreign server and the local server have to be stripped to get a neutral URL

Change History (5)

comment:1 Changed 11 years ago by ray

  • Resolution set to fixed
  • Status changed from new to closed

rev [992]

comment:2 Changed 11 years ago by guest

  • Resolution fixed deleted
  • Status changed from closed to reopened

This change introduced a bug. The "else" clause was accidentally stripped from the section of the fixRelativeLinks function that was edited in the changeset. The buggy version is:

    if ( typeof this.config.baseHref != 'undefined' && this.config.baseHref !== null )
    {
      baseRe = new RegExp( "((href|src|background|action)=\")(" + Xinha.escapeStringForRegExp(this.config.baseHref.replace(/([^\/]\/)(?=.+\.)[^\/]*$/, "$1")) + ")", 'g' );
    }
    baseRe = new RegExp( "((href|src|background|action)=\")(" +  Xinha.escapeStringForRegExp(document.location.href.replace( /^(https?:\/\/[^\/]*)(.*)/, '$1' )) + ")", 'g' );

Because the second baseRe= is no longer in an "else" clause, it effectively overwrites the first baseRe=, making the first case useless and preventing baseHref from being properly stripped. Please change to:

    if ( typeof this.config.baseHref != 'undefined' && this.config.baseHref !== null )
    {
      baseRe = new RegExp( "((href|src|background|action)=\")(" + Xinha.escapeStringForRegExp(this.config.baseHref.replace(/([^\/]\/)(?=.+\.)[^\/]*$/, "$1")) + ")", 'g' );
    }
    else
    {
      baseRe = new RegExp( "((href|src|background|action)=\")(" +  Xinha.escapeStringForRegExp(document.location.href.replace( /^(https?:\/\/[^\/]*)(.*)/, '$1' )) + ")", 'g' );
    }

Thanks! - Geoffrey

comment:3 Changed 11 years ago by guest

Perhaps I should add an explanation of the logic that I understand to be at work here.

The above snippet checks whether the configuration setting baseHref is undefined or empty. If it isn't undefined (let's say it contains "http://www.mydomain.org/filename.html"), then strip out "filename.html" (actually, do a few other complicated things with lookaheads to deal with special cases) and store what's left (the domain) as an escaped regular expression in baseRe -- which will be applied to all href, scr, etc. links (to remove the domain).

If baseHref is undefined or empty, however (the missing "else" clause), then construct a regular expression based on the document.location variable instead (after stripping out any filename.html). Store this in baseRe instead (that's the killer, because if this isn't in an else clause, it overwrites the first baseRe).

If the aim of this change was to allow setting of config.baseHref to a foreign domain, then I'm not sure this is the place to do it. The first regular expression above should work in that case. It would counstruct the required regex whether the domain set in baseHref is local or foreign (the regex used doesn't distinguish between them).

Sorry if the above is obvious to everyone, but I needed to write out the logic to get it straight in my own mind! - Geoffrey

comment:4 Changed 11 years ago by ray

  • Resolution set to fixed
  • Status changed from reopened to closed

rev [994]: The else was not accidentially removed, both RE's have to be used that's what the ticket was about. The error was that it was forgotten to do the replace with both

comment:5 Changed 11 years ago by ray

I should note that the reason I found both servers should be stripped is that with the "else"-construct in Firefox the URL is prefixed with the local server, so the image is not found, and in IE the URL is prefixed with the "baseHref-server" resulting in an absolute URL. So I thought easiest would be to strip everything, for the cost that one cannot explicitly link on the local server

Note: See TracTickets for help on using tickets.