Skip to main content
Topic: 1.1 RC 1 attachment problem (Read 2753 times) previous topic - next topic
0 Members and 1 Guest are viewing this topic.

1.1 RC 1 attachment problem

Hi,

my setting not allowed to see attachment  quest

pictures do not appear but looks like broken image

1.png

2.png
sorry for my bad english

Re: 1.1 RC 1 attachment problem

Reply #1

Check if guests have permissions to view attachments.   ;)

Re: 1.1 RC 1 attachment problem

Reply #2

Technically not exactly a bug, because it was not foreseen to have anything different, but probably not nice.

@Spuds @live627 the issue is Attachment_Controller throws an Elk_Exception('no_access', false) when the user is not allowed to see the attachment.
We should probably avoid exceptions in that controller and instead let it go and serve a dummy image? Maybe automagically generated using the localized string?

@badmonkey he said it in the first line. :P
Bugs creator.
Features destroyer.
Template killer.

Re: 1.1 RC 1 attachment problem

Reply #3

While the problem is in the chrome browser  
firefox no problem

@badmonkey  

It must be like in the pictures  


Re: 1.1 RC 1 attachment problem

Reply #4

It should display something to indicate its an access problem vs the broken image.

I'd opt for some default icon (padlock, circle with slash, grey blur image) with the title tag of $txt['cannot_view_attachments'] ?

QuoteWhile the problem is in the chrome browser  
firefox no problem
So that may indicate a problem with our header('Content-Disposition:  can you provide the link?


Re: 1.1 RC 1 attachment problem

Reply #6

I was looking into this one and I was taking the opportunity to cleanup a little the controller.
I stumbled upon this:
https://github.com/elkarte/Elkarte/blob/v1.1.0-RC.1/sources/controllers/Attachment.controller.php#L443
Code: [Select]
		// Make sure the mime type warrants an inline display.
if (isset($this->_req->query->image) && !empty($mime_type) && strpos($mime_type, 'image/') !== 0)
unset($this->_req->query->image);
// Does this have a mime type?
elseif (!empty($mime_type) && (isset($this->_req->query->image) || !in_array($file_ext, array('jpg', 'gif', 'jpeg', 'x-ms-bmp', 'png', 'psd', 'tiff', 'iff'))))
header('Content-Type: ' . strtr($mime_type, array('image/bmp' => 'image/x-ms-bmp')));
else
{
header('Content-Type: ' . (isBrowser('ie') || isBrowser('opera') ? 'application/octetstream' : 'application/octet-stream'));
if (isset($this->_req->query->image))
unset($this->_req->query->image);
}

1st condition looks good: if the request says "image", but the mime does not, believe the mime. (Apart that we do not send any header, that looks weird to me.)

The 2nd condition, instead I'm not sure what it wants to do.
If I read it correctly it says: if we have a mime and either request says image or the file extension is not that of an image, assume it is an image.

O_o

I agree we do not trust the request. Fine.
I wouldn't trust the file extension too, but... not consider the file an image if it has an image extension looks strange.

I went back to the commit from 2007 that first introduced this structure and couldn't get any hint because it was basically a rewrite of the previous check.

I was thinking to change it to something like:
Code: [Select]
elseif (!empty($mime_type) && in_array(str_replace('image/', '', $mime_type), array('jpeg', 'gif', 'png', 'bmp')))
so: we have mime and mime is one of the embeddable (jpeg, png, gif, bmp).
Bugs creator.
Features destroyer.
Template killer.

Re: 1.1 RC 1 attachment problem

Reply #7

I send a PR to fix this one.
I used always the "attachment not found" text, because is shorter than the "you don't have access" (otherwise the text would be almost invisible), that anyway is not that wrong.
Bugs creator.
Features destroyer.
Template killer.