use a simplate-specific file extension: .spt #148

Closed
whit537 opened this Issue Jan 24, 2013 · 13 comments

Projects

None yet

4 participants

@whit537
Owner
whit537 commented Jan 24, 2013

We may already have a ticket for this, but one of the things @dcrosta introduced in Keystone is the .ks file extension. This gets tacked on when URLs come in, so /foo.html in the browser resolves to /foo.html.ks on the filesystem. I've gotten another vote for this from @imathis and have half-considered it myself, actually. This adds value in a few ways (let's assume a .spt extension for simplates):

  • It makes the simplate resolution algorithm simpler: no more sniffing for control characters. "Explicit is better than implicit."
  • It's presumably easier to write syntax highlighting modes for text editors, etc.
  • It removes a hiccup for newcomers to Aspen when they open up a foo.json file and find ... Python?!
  • It potentially sets us up to implement tilt-style arbitrary transforms by popping extensions: foo.css.scss.spt -> foo.css.scss -> foo.css (cf. #112).

The flip side is that it complicates dispatching, and it means that you have to change the file extension on your file when you want to have simplate behavior. I suppose, though, that'd rarely be a problem: You could still have a foo.html.spt without any page breaks and it'd be served statically just like now.

Contributor
pjz commented Jan 24, 2013

Okay, but to me one common thing I've seen/used/wanted is a single simplate that renders to multiple types. What's that look like? just .spt ?

Owner
whit537 commented Jan 25, 2013

@pjz You mean negotiated simplates? Yeah, I would think that'd be foo.spt.

Owner
whit537 commented Jan 25, 2013

And per IRC if we have foo.html.spt and foo.html, we'll let foo.html.spt win.

Owner
whit537 commented Mar 27, 2013

Per gratipay/gratipay.com#800, this would help mitigate security vulnerabilities where application authors store user-uploaded files under the web publishing root. We could provide a helper for this case that ensures that user-uploaded files aren't saved with the .spt extension. We could also push the check out to the request parsing and 400 on any uploads with that file extension.

Contributor
pjz commented Mar 27, 2013

I'm willing to do the 'require a .spt extension' bit, but 1) I don't think it's that much of a security hole - if you're allowing people to upload to your web-publishing root, you're likely Doing It Wrong and will get bitten somehow anyway 2) Any helper and/or request parser would have to be used by the application author, and if they're smart enough to do that, then they'll likely not Do It Wrong in the first place. 3) If you really want those things done, I think they should be separate bugs.

@whit537 whit537 referenced this issue in gratipay/gratipay.com Mar 27, 2013
Closed

port to a different web framework #800

Owner
whit537 commented Mar 27, 2013

@pjz The request parsing wouldn't need to be explicitly called. We'd add checks on filenames for all uploads.

I think the use case is not that uncommon. If you're uploading files, you almost certainly want them to be downloadable again through the same app, no? Wouldn't it be natural to expose those through the www root?

Contributor
pjz commented Mar 27, 2013

I... guess? I'm paranoid too, and IMO you shouldn't do that.. you should write the files to some other dir, then have a /whatever/download/path/%filename simplate that looks in that other dir for the file named filename ; after properly sanitizing filename.

I'm a little worried about the performance impact of checking 'all uploads'... inspecting every set of POST data seems likely to slow things down. Also, full checking is actually a turing-complete problem: consider a form with 'filename' and 'content' fields that the idiot app developer blithely writes to disk.

just dropping this here:

the single reason why i didn’t look twice at aspen when i first came across it is that i can’t write a simple highlighting file for my editor of choice due to complex determination if it’s really a simplate.

adding that complex logic in every editor is quite much considering that the reason of no dedicated extension was mainly a stylistic choice.

👍

@whit537 whit537 added a commit that referenced this issue Apr 15, 2013
@whit537 whit537 Start hacking on #148
I made a couple changes in the resource machinery and started in on
tests, and realized that this gets into the dispatch machinery.
b656897
@whit537 whit537 added a commit that referenced this issue Apr 15, 2013
@whit537 whit537 Prune set_trace call; #148 e3e2063
@pjz pjz added a commit that referenced this issue Apr 16, 2013
@pjz pjz .spt implementation to fix #148 6b70e10
Contributor
pjz commented Apr 16, 2013

That should do it. Review it, test it, merge it if you dare.

@whit537 whit537 added a commit that referenced this issue Apr 27, 2013
@whit537 whit537 Get tests kind of passing for socket files; #148
These three tests in test_sockets_.py pass now, but one takes a few
seconds when I run that file in isolation, and hangs forever when I make
test. Wuh?!
c81b96e
@whit537 whit537 added a commit that referenced this issue Apr 27, 2013
@whit537 whit537 Only init mimetypes if it isn't already; #148
See comment inline for more info. This is a workaround for a big test
slowdown that we're seeing between master and the spt branch. Need to
look into that.
5410aa1
@whit537 whit537 added a commit that referenced this issue Apr 27, 2013
@whit537 whit537 Fix up the rest of the tests; #148
The tests are passing, though they're super slow. The only way they pass
at all for me is with that mimetypes.init conditional in the last
commit.
78d6d94
Owner
whit537 commented Apr 27, 2013

Check out this slow-down:

[aspen] $ make test
./env/bin/nosetests -sx tests/
..................................................................................................
..................................................................................................
..................................................................................................
..................................................................................................
..................................................................................................
..........
----------------------------------------------------------------------
Ran 500 tests in 27.946s

OK
[aspen] $ co master
Switched to branch 'master'
[aspen] $ make test
./env/bin/nosetests -sx tests/
..................................................................................................
..................................................................................................
..................................................................................................
..................................................................................................
..................................................................................................
..........
----------------------------------------------------------------------
Ran 500 tests in 3.781s

OK
[aspen] $
@whit537 whit537 added a commit that referenced this issue Apr 27, 2013
@whit537 whit537 Fix last socket test; #148
This test was originally breaking because we weren't properly starting a
socket thread because the filename was wrong and the content was wrong.
Now that the test is passing we needed to bring back the part where we
*close* the socket thread. That's where the slowdown was from. It's nice
though: the mimetypes.init thing still gives us almost a 4x boost in
test time. !m @pjz
c9da880
@pjz pjz added a commit that closed this issue Apr 27, 2013
@pjz pjz .spt implementation to fix #148 6b70e10
@pjz pjz closed this in 6b70e10 Apr 27, 2013
@whit537 whit537 added a commit that referenced this issue Apr 27, 2013
@whit537 whit537 Update comment around mimetypes.init; #148
We figured out the root cause of the slowdown that the mimetypes.init
change was discovered as a result of.
7c66ecf
@whit537 whit537 reopened this Apr 27, 2013
@whit537 whit537 closed this Apr 27, 2013
Owner
whit537 commented Apr 27, 2013

🍝

@whit537 whit537 added a commit that referenced this issue Apr 27, 2013
@whit537 whit537 Fix docs to refer to .spt; #148 85ef8de
@Changaco Changaco pushed a commit that referenced this issue Mar 11, 2016
@whit537 whit537 Start hacking on #148
I made a couple changes in the resource machinery and started in on
tests, and realized that this gets into the dispatch machinery.
9ef3409
@Changaco Changaco pushed a commit that referenced this issue Mar 11, 2016
@whit537 whit537 Prune set_trace call; #148 d6dc23c
@Changaco Changaco pushed a commit that referenced this issue Mar 11, 2016
@pjz pjz .spt implementation to fix #148 6bcf121
@Changaco Changaco pushed a commit that referenced this issue Mar 11, 2016
@whit537 whit537 Get tests kind of passing for socket files; #148
These three tests in test_sockets_.py pass now, but one takes a few
seconds when I run that file in isolation, and hangs forever when I make
test. Wuh?!
7c417f6
@Changaco Changaco pushed a commit that referenced this issue Mar 11, 2016
@whit537 whit537 Only init mimetypes if it isn't already; #148
See comment inline for more info. This is a workaround for a big test
slowdown that we're seeing between master and the spt branch. Need to
look into that.
d41dd5c
@Changaco Changaco pushed a commit that referenced this issue Mar 11, 2016
@whit537 whit537 Fix up the rest of the tests; #148
The tests are passing, though they're super slow. The only way they pass
at all for me is with that mimetypes.init conditional in the last
commit.
46cb2fd
@Changaco Changaco pushed a commit that referenced this issue Mar 11, 2016
@whit537 whit537 Fix last socket test; #148
This test was originally breaking because we weren't properly starting a
socket thread because the filename was wrong and the content was wrong.
Now that the test is passing we needed to bring back the part where we
*close* the socket thread. That's where the slowdown was from. It's nice
though: the mimetypes.init thing still gives us almost a 4x boost in
test time. !m @pjz
1eaa5f1
@Changaco Changaco pushed a commit that referenced this issue Mar 11, 2016
@whit537 whit537 Update comment around mimetypes.init; #148
We figured out the root cause of the slowdown that the mimetypes.init
change was discovered as a result of.
9ebeb1f
@Changaco Changaco pushed a commit that referenced this issue Mar 11, 2016
@whit537 whit537 Fix docs to refer to .spt; #148 08a4765
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment