Avatar crop screen filtering on wp_get_attachment_image_src () causes issues

[hnla]

BP appears to be filtering wp_get_attachment_image_src() to add 'avatar' to url path on the crop avatar screen, this causes problems for any images being fetched via attachment ID.

Scenario:
While working on a framework that sets or re-sets the default site logo using wp_get_attachment_image_src() I found that when I visited the upload avatar screen, uploaded a new avatar and then after being dropped back to the crop screen realised the site logo had vanished. On checking found the logo url was no longer '/uploads/2012/...' but now '/uploads/avatar/2012/...' thus unable to fetch non existent image at that path.

Further tested this on a plain WP/BP install and issue persisted

BP version 1.6 WP 3.4.1

To replicate or test:

Open header.php add:
<?php $src = wp_get_attachment_image_src( $attachment_id ); ?>
<p><?php echo $src[0] ?></p>

check path shown then navigate to upload new avatar, upload something to get to crop screen now check the url reported it should now read /uploads/avatar/...

[DJPaul]

Did you just notice this in 1.6, or did it work on older versions?

[boonebgorges]

Confirmed. The problem appears to be that we are filtering 'upload_dir' when handling avatar uploads, but then not removing the filter when we're done. Since we handle uploads before the page rendering starts, the filter then screws up any other upload URLs that are loaded from the template.

I'm kinda surprised this has never been noticed before. It takes a bit of an edge case to make it appear, but it's a pretty significant bug once you do get it to show up.

[boonebgorges]

(In [6812]) Remove avatar location filter after handling avatar uploads

BP filters upload_dir when handling avatar uploads, to ensure that avatar files
are stored in the proper directories. This changeset ensures that the filter is
removed after use, to ensure the integrity of calls to wp_upload_dir() later on
the crop page (after the avatar upload has been handled).

Fixes #4574

[hnla]

Apologies Paul lost track of this ticket ( relying on email notifications ) and missed your question.

Thanks Boone for following up. Yes it's definitely an edge case to replicate yet as you say surprised it never was stumbled on and as a resulting error it's rather a glaring one especially I would have thought on those sites that have massive deep headers to suddenly see big blank space :)

Out of interest I ran across this while working on Infinity, and I'll update the ticket I added on their github.

P.S it's an annoying feature of Trac that with the 'Awaiting Review' tag in operation even if you're the author when the ticket is accepted one doesn't get notified via email unless you have remembered to add yourself to the CC list (not usually necessary as the author)