Suppose you have a website accessible via HTTPS and/or via a non-standard port (e.g. 81). If you create an Iframe wrapper menu item pointing to a static page within your website by default the scheme "http://" and the current server name is prefixed to the given URL if it starts with "/".

You might test this with the static page "/logs/index.html". If you set up your web server such that you access all pages e.g. via port 81 only you'll get an error within your browser when loading the Iframe since it will try to load that page via port 80. Similarily when using HTTPS instead of HTTP the browser will load the Iframe via HTTP and it will probably complain about mixed content (insecure content within secure context).

Moreover I found an improper test for the schemes "http" and "https" in this context. The function strstr() returns a match for these strings even if they are embedded anywhere in the URL. I changed this test such that it now checks for "http://" or "https://" at the beginning of the URL by means of strpos().

I tested an iframe wrapper item with a relative url to my joomla page and it works with a non-standard port too. So the test was not successful, because I could not find the problem.

You're right, the non-standard port is part of $_SERVER['HTTP_HOST'], so running a Joomla web server at a non-standard port like port 81 alone will not cause problems with relative urls in the iframe wrapper. But what ist not covered correctly is the scheme (protocol) to be used when creating the absolute url from the relative url configured for the iframe wrapper. Currently this is "http://" since it is fixed within the code, and the scheme used for the page that embeds the iframe is not used.

So if you access your Joomla pages using https:// you'll get a security warning from your browser since it shall embed unsecure content into a secure web page.

My change, especially the change in the lines 86 and 87, shall change this behaviour. It will no longer use the protocol http:// together with the correct host name and port, but it will use the protocol of the page where the iframe wrapper is embedded.

The other change (line 89) shall fix the check whether the given url starts with either the http:// or the https:// protocol. The current version of this line simply checks whether either of the strings "http" or "https" exists in the url, and I think this check is not sufficient since these strings might exist anywhere in the url, and not necessarily at the beginning or the url.

Sorry for the confusion I might have caused with my comments regarding this change.

I have tested this item successfully on 8f3d2d3

Created new menu items as Wrapper with a relative url to an item inside the website.

Before: http://
After: https://

I have tested this item successfully on 8f3d2d3

New menu item -> Wrapper with internal relative path works with https

I have tested this item unsuccessfully on 8f3d2d3

If we add protocol relative URI (e.g. //www.example.com/path) it doesn't work properly.
For the rest works fine.

This PR has received new commits.

CC: @andrepereiradasilva, @euismod2336, @FPerisa, @insitevision

@andrepereiradasilva
A protocol relative URL didn't work with the original code nor with my improvements, the result is in both cases something like http://www.example.org//www.example.com/path where www.example.org is the current host name and //www.example.com/path is the URL given for the IFrame wrapper.
I now changed this behaviour such that a protocol relative URL (an URL starting with //) will be prepended with the current scheme (either http: or https:), moreover I changed the case that the given URL neither starts with http:// nor with https:// - in this case formerly the scheme http:// was prepended, now the current scheme is prepended too.

@jo-sf You tested protocol relative URL on the current staging?

@jo-sf See jo-sf#1

This PR has received new commits.

CC: @andrepereiradasilva, @euismod2336, @FPerisa, @insitevision

@wojsmol
I checked my patch against the current release (3.5.1) and I found no problems. The code affected by this patch hasn't been modified since my first pull request, due to that I don't expect any problems when merging this patch into the current code.
Thank you for your patch regarding the travis error - the leading blank was by mistake when I pasted the modified code into the editor.

@insitevision @andrepereiradasilva Can you please do your tests again? So we can move this PR forward. Thank you all.

Assuming Joomla is installed at https://www.website.com:90/joomla/.

Current patch is a nice improvement to the existing code.

Few things can still be improved/resolved in this:

1. http://www.anothersite.com      => unchanged (ok)
2. https://www.anothersite.com     => unchanged (ok)
3. //www.anothersite.com/page.html => https://www.anothersite.com/page.html (ok)

4. www.anothersite.com/page.html   => https://www.anothersite.com/page.html (ok)
5. contact-page.html               => https://contact-page.html (missed everything)
6. /contact-page.html              => https://www.website.com:90/contact-page.html (missed /joomla)

Once a standard guideline is given in the relevant section and implemented, this should be good to go.

IMO, it should be with these guidelines –

• If the url starts with single / it would be treated as domain root relative.
• The urls which doesn't start with / or // or http:// or https:// would be considered site-root relative.
• If the host (+port) is provided, then it is required to specify // or http:// or https:// as well. Otherwise they will be treated as relative to current site only.

Hence the effect would be:

4. www.anothersite.com/page.html   => https://www.website.com:90/joomla/www.anothersite.com/page.html (wrong usage)
5. contact-page.html               => https://www.website.com:90/joomla/contact-page.html (includes /joomla)
6. /contact-page.html              => https://www.website.com:90/contact-page.html (same as this patch)

I have tested this item ✅ successfully on 5cc6d18

I have tested this issue @icampus PBF with the proposed solution and can confirm that the fix works. Tested on Windows 7 with Firefox. Much like euismod2336 I created a Menu Item as a Wrapper with a relative URL and the link to the iframe switched (https to http) before the fix and applying the patch resolved the issue for me (it stayed https).

I have tested this item ✅ successfully on 5cc6d18

I created an Iframe Wrapper and set the url to "/logs/index.html", then acessed the site in the frontend once with "http" and "https".
Then I looked in the sourcecode and reproduced the issue that when i used "https", the iframe source is still "http://...".
So I applied the patch and tested it again, and it works.

I think the main issue is fixed, but in the comments i saw a few improvements that will be great to add to the patch.
Tested@icampus

RTC as we have 2 successful tests

@roland-d Please review against #4670 (comment) too, if that makes sense.

@izharaazmi That makes sense to me but I guess we need a separate PR for that. These guidelines are best to added to the comments in the code as well so in the future we know why things are done this way.

@izharaazmi There was a good reason why I implemented case 4 from #4670 (comment) as it is currently. I had browsers in mind where the URL bar shows no scheme but only the host name followed by the host relative path (at least if the scheme is HTTP), and if someone copies such an URL from the URL bar and uses this URL as the iframe target the content of this iframe target will load correctly if the correct scheme has been prepended. With your suggestion such an URL will always point to a non-existing page at the current host.

Merged with a905a01 - sorry it's taken so long to process this!