Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve old Content-Type behaviour for bundles #51

Merged
merged 1 commit into from
May 1, 2017

Conversation

quozl
Copy link
Contributor

@quozl quozl commented Apr 25, 2017

Problem: bundles downloaded with Mime-Type of application/octet-stream instead of application/vnd.olpc-sugar; breaking use of activities.sugarlabs.org. #48

Solution: detect downloads and change Mime-Type.

Regression introduced in Browse-200, did not occur in Browse-157.3.

@tony37
Copy link

tony37 commented Apr 25, 2017

This solution assumes downloads from ASLO. Activities can be downloaded from other sources such as a school server.

A more appropriate solution is for ASLO to identify activities with the correct mime_type.

@quozl
Copy link
Contributor Author

quozl commented Apr 26, 2017

A school server would use the right Content-Type header. Are you aware of any school servers that use application/octet-stream instead? Yes, ASLO might be changed, but with Sugar Network dependent on ASLO and heavily used by Peru there are risks of regression, so it is better to fix Browse so it works as it did before.

@tony37
Copy link

tony37 commented Apr 26, 2017 via email

@icarito
Copy link
Contributor

icarito commented Apr 28, 2017

Hi,
Thanks for considering the possibility of affecting Sugar Network. If this is the only reason to have a special case for activities.sugarlabs.org, then I agree with Tony that it would be better to fix ASLO instead. Now the thing is not many servers may know about application/vnd.olpc-sugar, do other browsers override mime from servers? It sounds like overkill and wasteful to guess MIME every time, if it isn't otherwise broken.

@icarito
Copy link
Contributor

icarito commented Apr 28, 2017

@tony37 that was GSOC 2016 ;-)

@quozl
Copy link
Contributor Author

quozl commented Apr 28, 2017

Good point. Yes, browsers vary in their handling of Content-Type header. Browsers may use, or not use, any of these sources of type;

  • server response Content-Type header, (e.g. application/octet-stream),
  • suffix of client request path, (e.g. the URL ending in .xo),
  • content bytes (e.g. using pattern recognition, like magic(5) or file(1))

There is no commonly accepted standard; but using the same method as the most popular browsers may be a good starting point.

Gio.content_type_guess uses file path and content bytes.

Browse-157.3 (WebKit API) effectively uses Gio.content_type_guess and ignores Content-Type.

Browse-200 (WebKit2 API) effectively uses Content-Type only, defaulting to whatever WebKit2 chooses, which for ASLO is application/octet-stream.

On the assumption that we cannot fix ASLO; I'm inclined to remove the conditional check for activities.sugarlabs.org and call Gio.content_type_guess always, and if the return is application/vnd.olpc-sugar don't use Content-Type header.

Should ASLO and mirrors be fixable; Browse-200 will only be broken with web repositories of activities that don't have the correct type in web server configuration. So there's still a small reason to fix Browse no matter what happens with ASLO; if we want Browse-200 to work in the same way as Browse-157.3.

@tony37
Copy link

tony37 commented Apr 28, 2017 via email

@quozl
Copy link
Contributor Author

quozl commented Apr 29, 2017

Yes, that's what I said; Browse-157.3 did that, Browse-200 does not.

I've researched to assess impact; affected now are users of Sugar on downstream distributions; Debian, Ubuntu, or Fedora. They have already packaged Browse-200. Latest SoaS is affected. No impact on OLPC OS because I'll be carrying a patch relative to upstream.

@quozl quozl force-pushed the 2017-115-bundles branch from bb41a38 to d1fa67f Compare May 1, 2017 09:45
@quozl quozl changed the title Guess Mime-Type for activities.sugarlabs.org Preserve old Content-Type behaviour for bundles May 1, 2017
@quozl
Copy link
Contributor Author

quozl commented May 1, 2017

Rewritten; new commit description of d1fa67f as follows;

Preserve old Content-Type behaviour for bundles

Problem: bundles may be provided by servers with a Content-Type of application/octet-stream instead of application/vnd.olpc-sugar, which when passed into the Journal prevents quick bundle installation.

Solution: restore Browse-157.3 behaviour by detecting bundles and ignoring the Content-Type from the server.

Regression introduced in Browse-200, did not occur in Browse-157.3.

Problem: bundles may be provided by servers with a Content-Type of
application/octet-stream instead of application/vnd.olpc-sugar, which
when passed into the Journal prevents quick bundle installation.

Solution: restore Browse-157.3 behaviour by detecting bundles and
ignoring the Content-Type from the server.

Regression introduced in Browse-200, did not occur in Browse-157.3.

References:
https://bugs.sugarlabs.org/ticket/4977

Signed-off-by: James Cameron <[email protected]>
Tested-by: Frederick Grose <[email protected]>
@quozl quozl force-pushed the 2017-115-bundles branch from d1fa67f to c82eda8 Compare May 1, 2017 22:30
@quozl
Copy link
Contributor Author

quozl commented May 1, 2017

Changed commit description to add Tested-by: Frederick Grose [email protected], and bug 4977.

@quozl quozl merged commit 8fe2509 into sugarlabs:master May 1, 2017
@quozl quozl deleted the 2017-115-bundles branch May 1, 2017 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants