[3.0] Recognise a gallery avatar as a file name, not a url - #9442
Open
albertlast wants to merge 1 commit into
Open
[3.0] Recognise a gallery avatar as a file name, not a url#9442albertlast wants to merge 1 commit into
albertlast wants to merge 1 commit into
Conversation
An avatar picked out of the gallery is stored as a path relative to the avatars directory: "Oxygen/cards.png", not a url. Avatar had no case for that, so it fell through to the last resort, which pulls the path out of the url and looks for the file under the avatar directories relative to the board directory. That only lines up when the forum is installed under a path prefix; at the root of a domain it reads Url::create(Config::$boardurl)->path, which is a typed property that was never assigned, and throws. So a forum at the root of its domain was a fatal error on every page showing a member with a gallery avatar - the board index included, by way of the last-post line - and everywhere else the avatar came out as default.png. It also meant the choice never stuck. Profile::setAvatarServerStored() puts the right value in place, but saving a member runs it back through Avatar, and since that could not resolve it, the column was written with whatever it had settled for instead. A string with no scheme is a file name. Saying so before the search starts lets it match on the second attempt, where the prepackaged avatar directory is already handled. Signed-off-by: Mathias Alberts <mathiaspapealbert@hotmail.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
An avatar picked out of the gallery is stored as a path relative to the avatars directory —
Oxygen/cards.png— not as a url.Avatarhas no case for that, so it falls through to itslast resort, which pulls the path out of the url and looks for the file under the avatar
directories relative to the board directory.
That only lines up when the forum is installed under a path prefix. Two things go wrong
otherwise:
Every page is a fatal error. At the root of a domain there is no path to strip, and the
code reads it anyway:
Url::$pathis typed with no default and was never assigned, so:One member with a gallery avatar takes down the board index, every topic they posted in,
the memberlist and their profile.
And the choice never sticks.
Profile::setAvatarServerStored()puts the right value inplace — verified,
new_data['avatar'] = 'Oxygen/cards.png'— but saving a member runs itback through
Avatar, and since that cannot resolve it,User::updateMemberData()writesthe column from whatever the object settled for instead. Set by hand, the avatar rendered
as
default.png, andAvatar::$choicereportednone, so the picker came back withNo avatar selected.
A string with no scheme is a file name. Saying so before the search starts lets it match on
attempt 2, where the prepackaged avatar directory is already handled, and the last-resort
url guessing is never reached.
Checked
http://localhost:8080— a forum at the root of its domain.Before, with
smf_members.avatar=Oxygen/bug.png:After, saving each choice in turn through Profile → Forum Profile and re-reading the
profile:
Oxygen/cookie.png)Oxygen/cookie.png…/avatars/Oxygen/cookie.pngdefault.png)''…/avatars/default.png''…/avatars/default.pnggravatar://secure.gravatar.com/avatar/…Oxygen/bug.png)Oxygen/bug.png…/avatars/Oxygen/bug.pngsmf_log_errorsstayed empty throughout. The top-leveldefault.pngstoring as''isexisting behaviour —
User::updateMemberData()treats the default image as "no avatar".This also settles the crash that #9440 guards against, by never reaching that branch for a
gallery avatar. #9440 is still worth having: it is the defensive fix for anything else that
gets that far with a pathless forum url.
Issues References (Fixes|Related|Closes)
Related #9440, #9441