Add app engine app to mirror html docs from chromium.googlesource.com

Add app engine app to mirror html docs from chromium.googlesource.com

BUG=crashpad:67
R=mark@chromium.org, rsesek@chromium.org

Committed: https://chromium.googlesource.com/crashpad/crashpad/+/a2740b23a2b5a8aaa65a8323fe784b5cbf14377d

[Bons, 2015-10-08 18:16:50]

andybons@chromium.org changed reviewers:
+ mark@chromium.org, rsesek@chromium.org

[Bons, 2015-10-08 18:16:51]

HUGE COMPLICATED CL

[Robert Sesek, 2015-10-08 18:27:27]

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go
File docs-appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go#newc...
docs-appengine/main.go:34: http.Redirect(w, r, "/doc/index.html",
http.StatusSeeOther)
303 is typically used in response to a POST request. I'd use 302 (Found)
instead. MovedPermanently is a candidate, but since we'll probably change the
URL structure after we move the docs off googlecode, Found is probably a better
choice.

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go#newc...
docs-appengine/main.go:38: resp, err := client.Get(baseURL + r.URL.Path)
Prevent path traversal in Path.

[Mark Mentovai, 2015-10-08 18:28:03]

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/app.yaml
File docs-appengine/app.yaml (right):

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/app.yaml#new...
docs-appengine/app.yaml:1: application: crashpad-home
BUG=crashpad:67

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go
File docs-appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go#newc...
docs-appengine/main.go:1: // Copyright 2015 The Crashpad Authors. All rights
reserved.
Ought to be in doc/appengine

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go#newc...
docs-appengine/main.go:34: http.Redirect(w, r, "/doc/index.html",
http.StatusSeeOther)
This one will become just index.html.

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go#newc...
docs-appengine/main.go:36: }
/man or /man/ or both should become /man/index.html too.

Or, to cover both of these, if we detect we’re getting back a directory from
git, we can tack on index.html ourselves. That’s nice and flexible, and it’ll
cover /doxygen/ too, although we don’t currently have any bare links to
/doxygen/.

[Bons, 2015-10-08 18:55:18]

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/app.yaml
File docs-appengine/app.yaml (right):

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/app.yaml#new...
docs-appengine/app.yaml:1: application: crashpad-home
On 2015/10/08 18:28:03, Mark Mentovai wrote:
> BUG=crashpad:67

Done.

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go
File docs-appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go#newc...
docs-appengine/main.go:1: // Copyright 2015 The Crashpad Authors. All rights
reserved.
On 2015/10/08 18:28:03, Mark Mentovai wrote:
> Ought to be in doc/appengine

Done.

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go#newc...
docs-appengine/main.go:34: http.Redirect(w, r, "/doc/index.html",
http.StatusSeeOther)
On 2015/10/08 18:27:27, Robert Sesek wrote:
> 303 is typically used in response to a POST request. I'd use 302 (Found)
> instead. MovedPermanently is a candidate, but since we'll probably change the
> URL structure after we move the docs off googlecode, Found is probably a
better
> choice.

Done.

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go#newc...
docs-appengine/main.go:34: http.Redirect(w, r, "/doc/index.html",
http.StatusSeeOther)
On 2015/10/08 18:28:03, Mark Mentovai wrote:
> This one will become just index.html.

Will update once we have the repo url

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go#newc...
docs-appengine/main.go:36: }
On 2015/10/08 18:28:03, Mark Mentovai wrote:
> /man or /man/ or both should become /man/index.html too.
> 
> Or, to cover both of these, if we detect we’re getting back a directory from
> git, we can tack on index.html ourselves. That’s nice and flexible, and it’ll
> cover /doxygen/ too, although we don’t currently have any bare links to
> /doxygen/.

ditto when we can test on the real repo url

https://codereview.chromium.org/1393353002/diff/1/docs-appengine/main.go#newc...
docs-appengine/main.go:38: resp, err := client.Get(baseURL + r.URL.Path)
On 2015/10/08 18:27:27, Robert Sesek wrote:
> Prevent path traversal in Path.

app engine seems to be doing this already? o_O ¯\_(ツ)_/¯

[Mark Mentovai, 2015-10-08 19:06:00]

We have https://chromium.googlesource.com/crashpad/crashpad/+/doc/doc/generated/
set up and working now.

[Mark Mentovai, 2015-10-08 19:07:09]

https://codereview.chromium.org/1393353002/diff/40001/doc/appengine/main.go
File doc/appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/40001/doc/appengine/main.go#n...
doc/appengine/main.go:52: w.Header().Set("Content-Type",
resp.Header.Get("Content-Type"))
We have .html, .css, .js, and .png. Does this set the right Content-Type for all
of these?

[Bons, 2015-10-08 21:29:56]

OK. take another look. Memcache code is up.

https://codereview.chromium.org/1393353002/diff/80001/doc/appengine/app.yaml
File doc/appengine/app.yaml (right):

https://codereview.chromium.org/1393353002/diff/80001/doc/appengine/app.yaml#...
doc/appengine/app.yaml:6: handlers:
the browser will always request the favicon. don't forward it to the server.

[Mark Mentovai, 2015-10-08 21:46:17]

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/app.yaml
File doc/appengine/app.yaml (right):

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/app.yaml...
doc/appengine/app.yaml:7: - url: /favicon\.ico
I’d rather serve this out of git too, so that the doc/generated tree matches
exactly what gets served without any magic like this.

We can check favicon.ico into doc/support, and add a cp doc/support/favicon.ico
"${output_dir}/" to doc/support/generate.sh. Then it should just be a matter of
setting the right MIME type in main.go.

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/main.go
File doc/appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/main.go#...
doc/appengine/main.go:74: w.Header().Set("Content-Type", "text/html")
If you’re going to copy the response: carry the status code over to the response
too?

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/main.go#...
doc/appengine/main.go:87: Expiration: 24 * time.Hour,
Maybe that’s a little long, is one hour OK? Robert’s going to want to see his
changes reflected quickly.

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/main.go#...
doc/appengine/main.go:98: w.Header().Set("Content-Type",
contentType(path.Base(u.Path)))
Sensible to set Content-Length too, since I think we’ll always know it at this
point?

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/main.go#...
doc/appengine/main.go:102: var contentTypes = map[string]string{
I don’t know anything about Go, but should this be inside the contentType
function?

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/main.go#...
doc/appengine/main.go:113: return typ + "; charset=UTF-8"
charset doesn’t make sense for png.

https://codereview.chromium.org/1393353002/diff/120001/doc/appengine/main.go
File doc/appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/120001/doc/appengine/main.go#...
doc/appengine/main.go:75: w.Header().Set("Content-Type", "text/html")
If you’re going to copy the response: carry the Content-Type from the server
too?

https://codereview.chromium.org/1393353002/diff/120001/doc/appengine/main.go#...
doc/appengine/main.go:76: io.Copy(w, resp.Body)
Is it actually kosher to do this? Might leak something we don’t want to expose
publicly.

[Bons, 2015-10-08 22:31:17]

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/main.go
File doc/appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/main.go#...
doc/appengine/main.go:74: w.Header().Set("Content-Type", "text/html")
On 2015/10/08 21:46:16, Mark Mentovai wrote:
> If you’re going to copy the response: carry the status code over to the
response
> too?

Done.

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/main.go#...
doc/appengine/main.go:87: Expiration: 24 * time.Hour,
On 2015/10/08 21:46:17, Mark Mentovai wrote:
> Maybe that’s a little long, is one hour OK? Robert’s going to want to see his
> changes reflected quickly.

Done.

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/main.go#...
doc/appengine/main.go:98: w.Header().Set("Content-Type",
contentType(path.Base(u.Path)))
On 2015/10/08 21:46:16, Mark Mentovai wrote:
> Sensible to set Content-Length too, since I think we’ll always know it at this
> point?

that’s done automatically.

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/main.go#...
doc/appengine/main.go:102: var contentTypes = map[string]string{
On 2015/10/08 21:46:16, Mark Mentovai wrote:
> I don’t know anything about Go, but should this be inside the contentType
> function?

Yeah. Moved.

https://codereview.chromium.org/1393353002/diff/100001/doc/appengine/main.go#...
doc/appengine/main.go:113: return typ + "; charset=UTF-8"
On 2015/10/08 21:46:16, Mark Mentovai wrote:
> charset doesn’t make sense for png.

Done.

[Mark Mentovai, 2015-10-08 23:35:47]

LGTM from a non-Go-reader

https://codereview.chromium.org/1393353002/diff/160001/doc/appengine/main.go
File doc/appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/160001/doc/appengine/main.go#...
doc/appengine/main.go:114: ".ico":  "x-icon",
image/x-icon, I think.

https://codereview.chromium.org/1393353002/diff/160001/doc/appengine/main.go#...
doc/appengine/main.go:114: ".ico":  "x-icon",
You still need to update doc/support/generate.sh to do

  cp doc/favicon.ico "${output_dir}/"

Can you put that right after where it moves index.html and right before it
builds man/index.html?

[Bons, 2015-10-09 02:37:54]

https://codereview.chromium.org/1393353002/diff/160001/doc/appengine/main.go
File doc/appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/160001/doc/appengine/main.go#...
doc/appengine/main.go:114: ".ico":  "x-icon",
On 2015/10/08 23:35:46, Mark Mentovai wrote:
> image/x-icon, I think.

Yep. Done.

https://codereview.chromium.org/1393353002/diff/160001/doc/appengine/main.go#...
doc/appengine/main.go:114: ".ico":  "x-icon",
On 2015/10/08 23:35:46, Mark Mentovai wrote:
> You still need to update doc/support/generate.sh to do
> 
>   cp doc/favicon.ico "${output_dir}/"
> 
> Can you put that right after where it moves index.html and right before it
> builds man/index.html?

Done.

[Mark Mentovai, 2015-10-09 03:11:30]

LGTM

[Robert Sesek, 2015-10-09 16:23:05]

LGTM

[Mark Mentovai, 2015-10-09 16:26:05]

https://codereview.chromium.org/1393353002/diff/180001/doc/appengine/main.go
File doc/appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/180001/doc/appengine/main.go#...
doc/appengine/main.go:58: if strings.HasSuffix(r.URL.Path, "/") {
Will we be able to detect that something was a directory based on the server’s
response if the client request didn’t end with /?

[Bons, 2015-10-09 16:31:34]

https://codereview.chromium.org/1393353002/diff/180001/doc/appengine/main.go
File doc/appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/180001/doc/appengine/main.go#...
doc/appengine/main.go:58: if strings.HasSuffix(r.URL.Path, "/") {
On 2015/10/09 16:26:05, Mark Mentovai wrote:
> Will we be able to detect that something was a directory based on the
server’s
> response if the client request didn’t end with /?

We can do a check to see if it looks like this:
https://crashpad-home.appspot.com/doxygen

how hacky do you want the code to be?

[Mark Mentovai, 2015-10-09 16:41:19]

https://codereview.chromium.org/1393353002/diff/180001/doc/appengine/main.go
File doc/appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/180001/doc/appengine/main.go#...
doc/appengine/main.go:58: if strings.HasSuffix(r.URL.Path, "/") {
On 2015/10/09 16:31:34, Bons wrote:
> On 2015/10/09 16:26:05, Mark Mentovai wrote:
> > Will we be able to detect that something was a directory based on the
> server’s
> > response if the client request didn’t end with /?
> 
> We can do a check to see if it looks like this:
> https://crashpad-home.appspot.com/doxygen
> 
> how hacky do you want the code to be?

Less hacky than that if possible.

I did a couple of test requests against Gitiles and it seems to return
“X-Gitiles-Object-Type: tree” for directories. It’s “blob” for files.

[Bons, 2015-10-09 19:20:31]

On 2015/10/09 16:41:19, Mark Mentovai wrote:
> https://codereview.chromium.org/1393353002/diff/180001/doc/appengine/main.go
> File doc/appengine/main.go (right):
> 
>
https://codereview.chromium.org/1393353002/diff/180001/doc/appengine/main.go#...
> doc/appengine/main.go:58: if strings.HasSuffix(r.URL.Path, "/") {
> On 2015/10/09 16:31:34, Bons wrote:
> > On 2015/10/09 16:26:05, Mark Mentovai wrote:
> > > Will we be able to detect that something was a directory based on the
> > server’s
> > > response if the client request didn’t end with /?
> > 
> > We can do a check to see if it looks like this:
> > https://crashpad-home.appspot.com/doxygen
> > 
> > how hacky do you want the code to be?
> 
> Less hacky than that if possible.
> 
> I did a couple of test requests against Gitiles and it seems to return
> “X-Gitiles-Object-Type: tree” for directories. It’s “blob” for files.

Done.

[Mark Mentovai, 2015-10-09 19:26:17]

https://codereview.chromium.org/1393353002/diff/240001/doc/appengine/main.go
File doc/appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/240001/doc/appengine/main.go#...
doc/appengine/main.go:79: http.Redirect(w, r, path.Join(r.URL.Path,
"/index.html"), http.StatusFound)
If the client requested /doc/, will they be sent to /doc/index.html (good) or
/doc//index.html (weird)?

[Bons, 2015-10-09 19:31:16]

https://codereview.chromium.org/1393353002/diff/240001/doc/appengine/main.go
File doc/appengine/main.go (right):

https://codereview.chromium.org/1393353002/diff/240001/doc/appengine/main.go#...
doc/appengine/main.go:79: http.Redirect(w, r, path.Join(r.URL.Path,
"/index.html"), http.StatusFound)
On 2015/10/09 19:26:17, Mark Mentovai wrote:
> If the client requested /doc/, will they be sent to /doc/index.html (good) or
> /doc//index.html (weird)?

path.Join normalizes the paths so that // won't happen.

[Mark Mentovai, 2015-10-09 19:33:34]

LGTM

[Mark Mentovai, 2015-10-09 19:59:37]

Committed patchset #13 (id:240001) manually as
a2740b23a2b5a8aaa65a8323fe784b5cbf14377d (presubmit successful).