Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Conversation

@jcansdale
Copy link
Collaborator

@jcansdale jcansdale commented Apr 3, 2018

This PR limits the number of attempts that will be made to download avatar when a host requires authentication (which isn't currently supported for images).

GitHub Enterprise returns a 200/OK with a content type of text/html when authentication is required to download an avatar.

What this PR does

  • When an avatar returns a non-image content type, remember this for subsequent calls to the same host

Reviewers

Todo

  • We should refine the type and/or number of exceptions that can be thrown before a host is blacklisted to make it more robust.
    • Changed to only remember exceptions caused by hosts returning non-image content

Fixes #1547

@jcansdale jcansdale changed the title [wip] Limit avatar download attempts Limit avatar download attempts Apr 3, 2018
@jcansdale jcansdale requested review from drguthals and grokys April 4, 2018 08:21
@jcansdale jcansdale removed the request for review from drguthals April 4, 2018 11:50
Copy link
Contributor

@grokys grokys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only problem with this is that it will still be throwing an exception for each requested avatar. That could be avoided by putting the blacklisting into AvatarProvider. However given our schedule and that this is a verified solution to a relatively serious problem, I'm 👍.

@grokys grokys merged commit b755fca into master Apr 11, 2018
@grokys grokys deleted the 1547/limit-avatar-download-attempts branch April 11, 2018 09:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Avatars being repeatedly downloaded.

3 participants