BscThesis / aboard.iee.ihu.gr

7 stars 0 forks source link

Attachments API #5

Closed asidirop closed 3 years ago

asidirop commented 3 years ago

We should add the route /api/announcements/{announcement_id}/attachments/{attachment_id} which should return the file itself.

Since {attachment_id} is under {announcement_id}, the permissions of announcement should be checked.

After that, the data of the attachemnt should not be included in the announcement_id responce (/api/announcements/{announcement_id}) and the interface should be updated in order to show the correct links for download.

(priority B)

nickcn commented 3 years ago

What I fail to understand here is how are we supposed to "present" the attachment itself? Should I implement it in the way that is currently used by apps (e.g. redirect to /api/announcements/{announcement_id}/attachments/{attachment_id})? Would we prefer another way (e.g full-screen modal that presents the attachment)?

asidirop commented 3 years ago

Just a link to the attachment from the announcement.... <a href='https:/..../api/announcements/234/attachments/34543'> ``Instead of link of type <a href='data:......'>

nickcn commented 3 years ago

What would the api return in this case?

asidirop commented 3 years ago

The binary contents of the attachment.

Antonis Sidiropoulos

Στις Τετ, 14 Απρ 2021, 22:26 ο χρήστης Nikolaos Christos Nikolaidis < @.***> έγραψε:

What would the api return in this case?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/BscThesis/aboard.iee.ihu.gr/issues/5#issuecomment-819774287, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKW7XAQEM37NWIONFTPS2LTIXUAFANCNFSM42GVORQA .

asidirop commented 3 years ago

Example:

<?php 
// include database connection 
include "db_connect.php"; 

// select the image 
$query = "select * from images WHERE id = ?"; 
$stmt = $con->prepare( $query );

// bind the id of the image you want to select
$stmt->bindParam(1, $_GET['id']);
$stmt->execute();

// to verify if a record is found
$num = $stmt->rowCount();

if( $num ){
    // if found
    $row = $stmt->fetch(PDO::FETCH_ASSOC);

    // specify header with content type,
    // you can do header("Content-type: image/jpg"); for jpg,
    // header("Content-type: image/gif"); for gif, etc.
    header("Content-type: image/png");

    //display the image data
    print $row['data'];
    exit;
}else{
    //if no image found with the given id,
    //load/query your default image here
}
?>
nickcn commented 3 years ago

I was not aware of this. Live and learn. :) We already have the mime type in the db. It would not cause any trouble with the api? It will simply return the file?

asidirop commented 3 years ago

No. Not at all.

Also do not forget to set the filename, mimetype and size in http response header as: Content-Disposition: attachment; filename="lala" Content-Type: lolo Content-Length: koko

The filename="lala" will set the default filename for saving.

nickcn commented 3 years ago

See this as a reference: http://aboard.iee.ihu.gr/api/announcements/22275/attachments/1372. Please observe the headers. Seems ok to me. The name in Greek is gibberish. Shall I add another function to transform the name to use Latin characters?

nickcn commented 3 years ago

The method used is checkout at the branch attachments_api. Not the best one out there. :)

asidirop commented 3 years ago

You have to remove the quotes from Content-Disposition

HTTP/1.1 200 OK Server: nginx Content-Type: application/pdf Content-Length: 683182 Connection: keep-alive Cache-Control: no-cache, private Date: Wed, 21 Apr 2021 07:53:03 GMT Content-Disposition: "attachment; filename="9ο Newsletter_ΟΜΠΡΕΛΑ.pdf"

asidirop commented 3 years ago

Use this for utf8: header('Content-Disposition: attachment;' . 'filename="' . addslashes(utf8_decode($filename)) . '";' . 'filename*=utf-8\'\'' . rawurlencode($filename));

https://stackoverflow.com/questions/93551/how-to-encode-the-filename-parameter-of-content-disposition-header-in-http

nickcn commented 3 years ago

Without the quotes, it is just downloading the attachment. Is this the requested behaviour?

asidirop commented 3 years ago

Yes it is. We can also put a URL argument in the request: lalal/attachment/4343?action=view lalal/attachment/4343?action=download

Άν δοθεί το download, τότε θα βάζει filename.. Αν δοθεί view (ή τίποτε) (default view), τότε δεν θα βάζει to Content-Disposition, οπότε ο browser θα δείχνει το attachment.

Όταν θα δείχνουμε τα buttons για τα attachments εμφανίζουμε εικονίδιο "download", αν το content-type είναι pdf, τότε εμφανίζουμε και εικονίδιο "View".

nickcn commented 3 years ago

Αυτό το τελευταίο δεν μπορώ να το φανταστώ. Πού θα εμφανίζεται το εικονίδιο;

nickcn commented 3 years ago

Logic (besides security checks) is implemented. Please check. :)

asidirop commented 3 years ago

Yes.

Look how the double link mode could be image

Security: Everyone has access to an announcement has access to its attachments too. That it why a good design puts the attachment one level below the announcements. We prefer: http://aboard.iee.ihu.gr/api/announcements/22275/attachments/1372 than http://aboard.iee.ihu.gr/api/attachments/1372 which could be the same.

nickcn commented 3 years ago

The security logic for each additional route has to be reimplemented. So same checks from /api/announcements/X for /api/announcements/X/attachments/Y. I am working on it now. A good candidate for a middleware though.

nickcn commented 3 years ago

Checks are in place for the attachments route. Please check. If everything is ok, I should merge and we should close this.

nickcn commented 3 years ago

Everything is ready from my side for this.

nickcn commented 3 years ago

Merged.