Bug #69665
closed
Rendering of TypoScript shortcutIcon does not consider HTTPS (in some cases)
100%
Description
IMHO it should be possible to enter the URL for shortcutIcon as arbitrary URL, specifically in any the following formats:
- absolute path, starting with /
- absolute URL e.g. https://www.example.org
- scheme relative : //www.example.org
The current rendering is not flexible enough for HTTP/ HTTPS because the rendered and cached page will be dependant on the URL scheme first used. This may result in a URL with HTTP scheme used for favicon (if the page is first fetched via http), which will then be cached and used for HTTPS as well resulting in security warnings (mixed content).
TYPO3 source code (4.5 and 6.2 more or less the same):¶
if ($GLOBALS['TSFE']->pSetup['shortcutIcon']) {
$favIcon = $GLOBALS['TSFE']->tmpl->getFileName($GLOBALS['TSFE']->pSetup['shortcutIcon']);
if (is_file(PATH_site . $favIcon)) {
if (function_exists('finfo_open')) {
if ($finfo = @finfo_open(FILEINFO_MIME)) {
$iconMimeType = ' type="' . finfo_file($finfo, (PATH_site . $favIcon)) . '"';
finfo_close($finfo);
$pageRenderer->setIconMimeType($iconMimeType);
}
}
$pageRenderer->setFavIcon(GeneralUtility::getIndpEnv('TYPO3_SITE_URL') . $favIcon);
}
As you can see, it is not possible to use relative or absolute paths because TYPO3_SITE_URL is prepended in any case. It is not possible to use absolute URLs because this will result in is_file failing.
This issue has actually already been reported, but for typo3.org, see https://forge.typo3.org/issues/58265
Any help is appreciated.
Updated by Chris no-lastname-given over 8 years ago
Hi @ all? Is there no workaround available for this really bad issue? Or any target version?
Updated by Stephan Großberndt over 8 years ago
- Status changed from New to In Progress
- Assignee set to Stephan Großberndt
- Target version set to 7 LTS
- TYPO3 Version changed from 6.2 to 7
The cached header of a page is used as well for HTTP as HTTPS. If a page gets called and cached as HTTP first the next HTTPS page call generates a page which has a link to the favicon with http:// leading to mixed content and a insecure rating.
Updated by Gerrit Code Review over 8 years ago
- Status changed from In Progress to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/44213
Updated by Stephan Großberndt over 8 years ago
@chris and Sibylle Peters: Could you please verify the patch fixes the problem for you?
Updated by Gerrit Code Review over 8 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/44213
Updated by Gerrit Code Review over 8 years ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/44213
Updated by Gerrit Code Review over 8 years ago
Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/44213
Updated by Stephan Großberndt over 8 years ago
According to an Internet Explorer Developer IE7+? does not respect the <base href="" on a favicon. Thus to display one there this has to be an absolute URL.
http://jeffcode.blogspot.de/2007/12/why-doesnt-favicon-for-my-site-appear.html
This leaves us with different choices, each with its trade-offs:
1. Use a protocol relative URL (//example.com/foo/favicon.ico)- Not supported on IE6
- Not supported on IE7+?
- IE7+? correctly displays favicon
- Mixed content if page was rendered on HTTP and cached page header is used on HTTPS
- Overhead in the caching on the server
In my opinion mixed content is much worse than supporting totally outdated internet explorer versions.
Updated by Stephan Großberndt over 8 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 46d7856196e9d763bc6764e4f9687d58bcc825de.
Updated by Gerrit Code Review over 8 years ago
- Status changed from Resolved to Under Review
Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/44921
Updated by Gerrit Code Review over 8 years ago
Patch set 2 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/44921
Updated by Morton Jonuschat over 8 years ago
- Status changed from Under Review to Resolved
Applied in changeset 71b64be0b9e714d8b93ebace7fa6f87a1c37ea65.
Updated by Riccardo De Contardi over 6 years ago
- Status changed from Resolved to Closed
Updated by Stephan Großberndt over 6 years ago
Hello Sybille,
was this change just to test the functionality of changing the issue description or is there something to do here?
The patches are merged and included in published versions of TYPO3 so this should work.
Updated by Sybille Peters over 6 years ago
I corrected the formatting of some of my posts that looked bad. This was not a good idea if watchers are being notified. Did you get a notification email? If yes, I am sorry.