Fix header for RSS feeds with UTF8 characters. #3649

Merged
merged 1 commit into from Apr 14, 2015

Conversation

Projects
None yet
5 participants
@schneems
Contributor

schneems commented Apr 10, 2015

$ curl http://localhost:4000/feed.xml -I
HTTP/1.1 200 OK
Etag: 64af8c-2356-552805aa
Content-Type: text/xml; charset=utf-8
Content-Length: 9046
Last-Modified: Fri, 10 Apr 2015 17:17:30 GMT
Server: WEBrick/1.3.1 (Ruby/2.2.1/2015-02-26)
Date: Fri, 10 Apr 2015 17:17:34 GMT
Connection: Keep-Alive

Originally contributed to fitztrev/jekyll-utf8#3

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Apr 10, 2015

Contributor

Not sure if this is the best way to do this. It works...but it seems like a one off. I'm also not sure if you wanted "html" specified as well.

Contributor

schneems commented Apr 10, 2015

Not sure if this is the best way to do this. It works...but it seems like a one off. I'm also not sure if you wanted "html" specified as well.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Apr 10, 2015

Member

Idea: can this be specified in the mime.types file? Can it be global for all text files?

Member

parkr commented Apr 10, 2015

Idea: can this be specified in the mime.types file? Can it be global for all text files?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Apr 10, 2015

Member

I'm also not sure if you wanted "html" specified as well.

It's a good idea. 😄

Member

parkr commented Apr 10, 2015

I'm also not sure if you wanted "html" specified as well.

It's a good idea. 😄

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Apr 14, 2015

Member

Turns out this can be done in the mime.types file:

text/html;charset=utf-8               html htm shtml

produces:

{"html"=>"text/html;charset=utf-8", "htm"=>"text/html;charset=utf-8", "shtml"=>"text/html;charset=utf-8"}

🙌

Thoughts, @schneems?

Member

parkr commented Apr 14, 2015

Turns out this can be done in the mime.types file:

text/html;charset=utf-8               html htm shtml

produces:

{"html"=>"text/html;charset=utf-8", "htm"=>"text/html;charset=utf-8", "shtml"=>"text/html;charset=utf-8"}

🙌

Thoughts, @schneems?

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Apr 14, 2015

Member
<?xml version="1.0" encoding="utf-8"?>

The encoding specified in the document will override the encoding in the HTTP header. Fortunately, the encoding specified in the document is already UTF-8.

Member

pathawks commented Apr 14, 2015

<?xml version="1.0" encoding="utf-8"?>

The encoding specified in the document will override the encoding in the HTTP header. Fortunately, the encoding specified in the document is already UTF-8.

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Apr 14, 2015

Contributor

The encoding specified in the document will override the encoding in the HTTP header

It won't set a valid charset though. My feed wasn't being parsed as valid by w3c even with that https://github.com/schneems/schneems/blob/master/feed.xml

@parkr I like that approach better. Want me to update this PR?

Contributor

schneems commented Apr 14, 2015

The encoding specified in the document will override the encoding in the HTTP header

It won't set a valid charset though. My feed wasn't being parsed as valid by w3c even with that https://github.com/schneems/schneems/blob/master/feed.xml

@parkr I like that approach better. Want me to update this PR?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Apr 14, 2015

Member

@parkr I like that approach better. Want me to update this PR?

That would be amazing! 😄 Include it anywhere you think it's necessary.

Member

parkr commented Apr 14, 2015

@parkr I like that approach better. Want me to update this PR?

That would be amazing! 😄 Include it anywhere you think it's necessary.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Apr 14, 2015

Member

My feed wasn't being parsed as valid by w3c even with that https://github.com/schneems/schneems/blob/master/feed.xml

What you have here validates for me.

Member

pathawks commented Apr 14, 2015

My feed wasn't being parsed as valid by w3c even with that https://github.com/schneems/schneems/blob/master/feed.xml

What you have here validates for me.

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Apr 14, 2015

Contributor

@pathawks that was after schneems/schneems@4c70a5d it was failing before that.

Contributor

schneems commented Apr 14, 2015

@pathawks that was after schneems/schneems@4c70a5d it was failing before that.

Fix header for RSS feeds with UTF8 characters.
```
$ curl http://localhost:4000/feed.xml -I
HTTP/1.1 200 OK
Etag: 64af8c-2356-552805aa
Content-Type: text/xml; charset=utf-8
Content-Length: 9046
Last-Modified: Fri, 10 Apr 2015 17:17:30 GMT
Server: WEBrick/1.3.1 (Ruby/2.2.1/2015-02-26)
Date: Fri, 10 Apr 2015 17:17:34 GMT
Connection: Keep-Alive
```

Originally contributed to fitztrev/jekyll-utf8#3
@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Apr 14, 2015

Contributor

Updated PR

Contributor

schneems commented Apr 14, 2015

Updated PR

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Apr 14, 2015

Member

It looks like it is being served by GitHub pages; plugins are disabled.

Member

pathawks commented Apr 14, 2015

It looks like it is being served by GitHub pages; plugins are disabled.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Apr 14, 2015

Member

It looks like it is being served by GitHub pages; plugins are disabled.

@pathawks he runs it on heroku I think :)

Member

parkr commented Apr 14, 2015

It looks like it is being served by GitHub pages; plugins are disabled.

@pathawks he runs it on heroku I think :)

parkr added a commit that referenced this pull request Apr 14, 2015

@parkr parkr merged commit ab83f0b into jekyll:master Apr 14, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Apr 14, 2015

Member

http://dbtek.github.io/dbyll/feed.xml

Not served by github.io? 😶

Member

pathawks commented Apr 14, 2015

http://dbtek.github.io/dbyll/feed.xml

Not served by github.io? 😶

parkr added a commit that referenced this pull request Apr 15, 2015

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Apr 15, 2015

Member

Thank so much @schneems!!!!!! ❤️

Member

parkr commented Apr 15, 2015

Thank so much @schneems!!!!!! ❤️

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Apr 15, 2015

Contributor

My condolences to whoever uses WEBRick to server files on Heroku. I think it's time I build a Jekyll pack so that that kind of insanity is stopped before it causes a train wreck for somebody (worse than it already has.) It can't be that hard to run jekyll build and then boot Nginx on that.

Contributor

envygeeks commented Apr 15, 2015

My condolences to whoever uses WEBRick to server files on Heroku. I think it's time I build a Jekyll pack so that that kind of insanity is stopped before it causes a train wreck for somebody (worse than it already has.) It can't be that hard to run jekyll build and then boot Nginx on that.

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Apr 15, 2015

Contributor
$ dig www.schneems.com

; <<>> DiG 9.8.3-P1 <<>> www.schneems.com
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 34722
;; flags: qr rd ra; QUERY: 1, ANSWER: 3, AUTHORITY: 0, ADDITIONAL: 0

;; QUESTION SECTION:
;www.schneems.com.      IN  A

;; ANSWER SECTION:
www.schneems.com.   1252    IN  CNAME   schneems.herokuapp.com.

I work for Heroku.

Contributor

schneems commented Apr 15, 2015

$ dig www.schneems.com

; <<>> DiG 9.8.3-P1 <<>> www.schneems.com
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 34722
;; flags: qr rd ra; QUERY: 1, ANSWER: 3, AUTHORITY: 0, ADDITIONAL: 0

;; QUESTION SECTION:
;www.schneems.com.      IN  A

;; ANSWER SECTION:
www.schneems.com.   1252    IN  CNAME   schneems.herokuapp.com.

I work for Heroku.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Apr 15, 2015

Contributor

Whether you work for Heroku has no bearing on what I said at all.

Contributor

envygeeks commented Apr 15, 2015

Whether you work for Heroku has no bearing on what I said at all.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Apr 15, 2015

Contributor

Unless you guys don't want a build pack for Jekyll+Nginx for Heroku, then by all means say it.

Contributor

envygeeks commented Apr 15, 2015

Unless you guys don't want a build pack for Jekyll+Nginx for Heroku, then by all means say it.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Apr 15, 2015

Member

@schneems Are you using WEBrick? Isn't Puma recommended by Heroku? Or is it just not that big of a deal for you?

Member

parkr commented Apr 15, 2015

@schneems Are you using WEBrick? Isn't Puma recommended by Heroku? Or is it just not that big of a deal for you?

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Apr 15, 2015

Contributor

@parkr Puma can't serve static files sadly. It's WEBRick that is the problem but if we built a proper pack we could crush most of these problems before they happen because it has proper mimetypes.

Contributor

envygeeks commented Apr 15, 2015

@parkr Puma can't serve static files sadly. It's WEBRick that is the problem but if we built a proper pack we could crush most of these problems before they happen because it has proper mimetypes.

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Apr 15, 2015

Contributor

@envygeeks there's already a dedicated jekyll buildpack i've not used it. I would instead prefer a micro-buildpack of some kind. So you can use multi-buildpack with the regular Ruby buildpack, then run the microbuildpack after it. For serving assets you could use ActionDispatch::Static middleware if you really wanted.

Honestly latency hasn't been that bad, my perc 95 is around 150ms

I'm also running it on more than one dyno, so that helps.

Contributor

schneems commented Apr 15, 2015

@envygeeks there's already a dedicated jekyll buildpack i've not used it. I would instead prefer a micro-buildpack of some kind. So you can use multi-buildpack with the regular Ruby buildpack, then run the microbuildpack after it. For serving assets you could use ActionDispatch::Static middleware if you really wanted.

Honestly latency hasn't been that bad, my perc 95 is around 150ms

I'm also running it on more than one dyno, so that helps.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Apr 15, 2015

Contributor

I didn't know that was supported so I will definitely do that when I look into building a buildpack.

Contributor

envygeeks commented Apr 15, 2015

I didn't know that was supported so I will definitely do that when I look into building a buildpack.

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.