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

RFC: Server-side rendering #1173

Closed
wants to merge 5 commits into from

Conversation

seanlinsley
Copy link

This PR represents my efforts as a beginner Rust programmer to rewrite the UI so that it's rendered by the server, instead of by Ember in the browser.

For the pages converted so far, / and /categories, I refactored the existing JSON API functions so that they could be called by the server to be rendered by the handlebars Rust crate.

screen shot 2017-11-14 at 7 47 57 pmscreen shot 2017-11-17 at 1 53 03 pm

Is server-side rendering the right option?

I wasn't sure if this would be a welcome change, so I've only gone far enough to have a working prototype. Here's why I think it's worthwhile:

  • dogfooding
    • Crates.io could become a great example of how to build server-side apps, like Rubygems.org
    • this paves the way to write WebAssembly instead of JavaScript
  • ease of contribution
    • Ember is a great framework, but for such a simple app it just adds complexity & duplication
    • the burden on new contributors to learn Rust, Conduit, JavaScript, and Ember shouldn't be understated
  • performance
    • the main page takes roughly a second for assets to load & boot, then another second for API requests to finish
    • the API definitely needs to be optimized, but that first second of delay can only be removed with server-side rendering
  • security
    • browser scripting exposes you to risks sometimes as bad as the attacker gaining local root

Which web framework?

Currently we're using Conduit. There's a lot of options these days; does anyone have experience with those others, and would they be a better choice if we switch to server-side rendering?

Similarly, which Crates.io features might be so complex to hand-roll that it makes this conversion much more difficult? OAuth seems likely.

@seanlinsley seanlinsley mentioned this pull request Nov 20, 2017
@carols10cents
Copy link
Member

Hi Sean! First off, I want to thank you and express how much I appreciate you doing this work ❤️

With the backlog of other PRs I have, and with the holiday, it's going to take me some time to review, test, and consider the implications of this-- just a heads up that I do want to give this the consideration it deserves, but it might take me a while :)

@jtgeibel
Copy link
Member

Thanks a lot for putting some concrete code behind this concept @seanlinsley! Over the past few days I've been putting some thought into the tradeoffs here, and I'm not sure that server side rending as demonstrated here is the best way forward. I see you've linked the fastboot thread as well, and while I don't know the full details of what that path entails, I believe it would be the best way forward for progressively adding static HTML support.

I understand that with the developer focused user base of this site we likely have a higher than average percentage of users who surf the web with JS disabled. However, I think we need to be very careful about both how we address this use case and which functionality we choose to expose in this configuration. Eliminating frontend JS code doesn't necessarily result in better security for our users overall if it introduces complexity in the backend.

I'll give an example, but in general I fear that doing this incorrectly will increase our attack surface as we continue to support tools hitting the JSON API in addition to the new frontend HTML endpoints. In particular, I'm most worried about functionality that involves submitting data to the backend (via POST or PUT).

Finally, I think it is worth discussing the threat model we are addressing here. Allowing JS certainly opens the client up to bugs like the one you linked, but in that example the attacker is either crates.io itself or we have an XSS vulnerability. If crates.io is untrusted, then you're better off querying the index directly for the info when possible. If we have an XSS vulnerability, then even if we render to HTML on the server we still expose the majority of the user base that has JS enabled. While that is worse for the ecosystem overall, admittedly individual users with JS disabled are protected. (But can that user trust that all the developers of their dependencies are so vigilant?)

Security - CSRF Protection

The tools for preventing CSRF are different for <form> and XHR requests. When dealing with data submitted in a form, we would need to include a hidden field with a token that we verify when the form is submitted.

When dealing with XHR requests, it appears that we've largely dealt with this by using PUT endpoints (even in some cases where POST may be more semantically appropriate). I believe we should also be rejecting all requests that are not Content-Type: application/json but adding that would take care, especially for cargo endpoints.

Either of these cases (PUT or enforcing the content type) should be sufficient to protect us from rogue websites tricking users into submitting form data to our API, but we would need additional protections on any sensitive endpoints if we plan to allow JS-free support for: adding API tokens, adding/changing an email address, accepting owner invitations, following a crate, etc.

Because of this, I think any such dynamic functionality should be out of scope for our JS-free experience. We could support searching (a GET endpoint), but even there it gets a bit tricky when trying to support the sort-by dropdown.

Performance

I can see some scenarios where server side rending may slow down the page. For instance, if we want to show the readme and download chart in the static output for a crate, then we're probably waiting for all of that to finish so we can calculate the Content-Length. We could try streaming the response, but that is even more code to maintain.

Path Forward

Because of these concerns, I think we need to document a clear plan for which functionality we would support and how we progressively port that over to a static HTML experience without impacting the existing experience.

Frontend

As I understand it, fastboot looks like the best way forward for providing a static experience for users with JS disabled. It allows us to ship static HTML on the first page load and then for most users Ember loads and from there the experience is the same as today. If a user has JS disabled they would still see the page but the rest of Ember would not load. Links to other fastboot enabled pages would work (hitting the backend endpoints directly rather than being intercepted by Ember), but dynamic interactions would not.

I think we need to define a plan and set expectations for exactly what this experience looks like with JS disabled. For instance, can we hide the follow button or will the user see a button that doesn't work? (Back to performance, if we do implement this for static pages then that button will re-render a complex page on both the server and client. Today this is a simple API request, the frontend code is straightforward, and things are easily cached in a fine-grained way.)

Backend

On the backend, ideally a full-fledged web framework would handle CSRF, XSS, and other security concerns for us, but the path there isn't necessarily straightforward either. First, I'm not sure the ecosystem is stable enough for us to pick a winner at this point in time. We would lean heavily toward something that supports stable Rust today, but there are also a lot of interesting options that currently require nightly features.

An incremental step would probably be to port from conduit to hyper in the backend. A lot of frameworks seem to be built on hyper and changing HTTP backends can't be done incrementally. Once that is accomplished then it may be possible to progressively port over endpoints to a new framework.

More importantly, it would be nice to have clearly documented guidelines for our existing JSON API that could be used during the design and review of new functionality. We would also need to make sure that these are implemented correctly in the change to hyper and beyond.

Other

this paves the way to write WebAssembly instead of JavaScript

On a separate note, have you seen this series of tweets? I'm not sure exactly how Rust is being used in Glimmer.js, but it looks like the plan is to eventually make this available in Ember somehow. This may be a path toward shipping client side Rust functionality in the future.

@seanlinsley
Copy link
Author

I had security at the end of the list because it's (almost always) the least convincing argument 😄 I think that the new contributor experience & page load performance are much more important.

Re: there being an increased attack surface, I think it'd be perfectly reasonable to only have the JSON API for most non-GET requests, calling them via JavaScript as necessary. This would especially make sense if none of the existing web frameworks have matured to the point where they provide things like CSRF protection.

I haven't yet investigated those web frameworks. That's what I intend to work on next.

Re: the follow button ¯\(ツ)/¯ Mastodon implements an HTML-only follow button that sufficiently quickly reloads the page. Whether it needs JS is going to come down to whether the HTML version is fast enough for people to not notice.

Re: sending all the HTML at the very end possibly slowing things down, that's very true. The easiest mitigation is to still do an AJAX request for some data. In my experience with Rails, HTML streaming can work very well for this sort of problem; of course we probably shouldn't go that direction unless it's built into a web framework.

It's awesome to see Ember is looking at using Rust for its renderer ✨


This is a very early PR, to get feedback from y'all. Even if we all agree on a path forward, I don't expect it to get to a mergeable state in the next few months.

If there's enough support for this direction, I'd be happy to contribute features upstream to our web framework of choice.

@est31
Copy link
Member

est31 commented Nov 23, 2017

If we have an XSS vulnerability, then even if we render to HTML on the server we still expose the majority of the user base that has JS enabled.

One can disable javascript functionality even for people who have js disabled enabled (sorry for that mixup lol), with the help of CSP headers. Just send script-src 'none' :).

I can see some scenarios where server side rending may slow down the page.

There are far more scenarios where server side rendering speeds up rendering of the page. As browser, if you are getting the response by the site you can immediately start rendering and don't have to send an XHR request to get some further data needed, etc. I just compare the wikipedia page for obama with the crates.io page for winapi, and wikipedia renders faster even though it has a ton more content.

@seanlinsley
Copy link
Author

We would lean heavily toward something that supports stable Rust today, but there are also a lot of interesting options that currently require nightly features.

I'm pretty new to Rust; is the preference for stable Rust explained somewhere? Some things I found:

Rocket supports streaming, but I wouldn't be surprised if template streaming isn't yet available. It also doesn't yet support CSRF protection. Down the rabbit hole I go 😸

@est31
Copy link
Member

est31 commented Nov 26, 2017

Btw, in my own "local crates.io server" project https://github.com/est31/cargo-local-serve I am using iron as a web framework. It has a lot of cool stuff.

@jtgeibel
Copy link
Member

@seanlinsley:

I'm pretty new to Rust; is the preference for stable Rust explained somewhere?

I'm speaking more of crates.io than Rust in general. It really depends on your application. For crates.io specifically, I don't have a link to an official position but the major advantage I see is that it gives changes at least 6 (and up to 12) weeks between a bug hitting master and it shipping in stable.

I personally do my crates.io development on nightly and ran into rust-lang/rust#42903. If we compiled with nightly in production then this could have bit us there.

@est31:

Just send script-src 'none'

Fortunately, we're already in a pretty good place here. Our current policy includes: script-src 'self' 'unsafe-eval' https://www.google-analytics.com https://www.google.com;. Notably, we do not include 'unsafe-inline' so our exposure is limited to the eval call sites of scripts hosted (anywhere) on those whitelisted domains.

I just compare the wikipedia page for obama with the crates.io page for winapi, and wikipedia renders faster even though it has a ton more content.

The main performance issue I see is in booting the Ember application. The first navigation from an external site is a bit slow, but most navigation within the site is very performant. I think fastboot looks like a good way toward improving the worst pain-point of the current experience.

We do have some slow api endpoints. The summary endpoint on the homepage is an example. Maybe we can render the rest of the homepage while that loads.

I think the biggest potential performance pitfall with our architecture is for clients with high latency. Unfortunately, Heroku has no public plans for enabling HTTP/2 so requests come over several connections that individually ramp up. For mobile users this is probably more of an issue than updating the DOM from JS.

Btw, in my own "local crates.io server" project https://github.com/est31/cargo-local-serve I am using iron as a web framework. It has a lot of cool stuff.

Thanks for the link, looks interesting.

@est31
Copy link
Member

est31 commented Nov 28, 2017

Fortunately, we're already in a pretty good place here.

Yes, definitely, this is better than no CSP policy at all. Still, if you manage to upload something to crates.io as a script, you can do XSS attacks. With script-src 'none' that is prevented as well :).

For mobile users this is probably more of an issue than updating the DOM from JS.

If by "high latency" you mean ping time and not "low bandwith" you are having first one request to the site for the html, then a second request for the json, then requests for additional json files and even more data. A very large amount of the json being transmitted can be easily derived (e.g. download urls follow a very simple scheme). The big issue for high round trip time environments is not the size of the json however, it is the additional requests you are sending back and forth :). Ideally you do one request and then get all the data in reply you need to render the site.

@jtgeibel
Copy link
Member

The big issue for high round trip time environments is not the size of the json however, it is the additional requests you are sending back and forth :). Ideally you do one request and then get all the data in reply you need to render the site.

I agree, that's what I was trying to say. If we could enable HTTP/2 that would help most users here (unless they have old clients or are behind an HTTPS intercepting proxy).

Right now I'm seeing 9 json endpoint requests sent to crates.io when loading https://crates.io/crates/cargo-registry. If we aren't already, we should prioritize the important requests and render that information as soon as it is available.

@seanlinsley
Copy link
Author

I opened a feature request for template streaming support on Rocket: rwf2/Rocket#499, but it turns out that there's already a working prototype for Tera, one of the templating languages Rocket supports: Keats/tera#211.

@sunng87
Copy link
Contributor

sunng87 commented Dec 15, 2017

@seanlinsley Thank you for using handlebars-rust in this project. Feel free to let me know if you have any issue/question about the implementation.

@carols10cents
Copy link
Member

Ok I have done a lot of thinking on this. This would be a significant architecture change, and with all significant architecture changes, the code itself isn't actually the issue :) To be clear, again, I want to thank you for the work you put into this PR @seanlinsley, and the code looks great 👍 But I'm going to close this for now, and here's why:

Stability

Right now, crates.io works. It's an essential service for the Rust community, and we need to keep it working reliably. Alex Crichton and I are the main folks with access to the production Heroku instance. Alex does a lot of other things as well, and I'm about to have my first kid in April 👶 So for both of our sakes, I'd like to keep crates.io stable and functioning reliably in the near future.

That means the bar for making big architectural changes is set pretty high-- there needs to be very clear, large benefits and proof that we're not breaking anything. I'm not yet feeling confident enough about this direction to set you loose converting everything.

Some things that would make me more confident would be:

  • Performance tests in a browser with realistic data, comparing rendering time between the current codebase and one using server-side rendering, showing a dramatic improvement
  • Full stack tests covering at least the happy path of all the major features of crates.io, of which we currently have none (we have front-end tests that mock the API and back-end tests that check the API responses, but no tests that go through both components from the browser)

If you're up for doing that work rather than just conversion work over the next few months, that would be helpful-- but I wouldn't blame you if these aren't the things you're interested in working on :)

Fastboot

I know progress has been a long time coming on this, but the ember folks have told me we're only a few pull requests away from fastboot compatibility. This is a much smaller change than an architecture change, and I want to give fastboot a really good try in production before we decide it's not enough.

Interactivity

We're slowly adding more interactive client-side features that do lean on ember for what ember's good at, and I don't want to lose those. Some examples are:

  • Managing API tokens
  • Managing owners
  • Confirming email addresses

I don't want to go back to the jQuery days of inserting/removing DOM nodes for this sort of interactivity, I like the APIs ember has for this :)

I'd be interested to see whether we'd need to transition to doing a full page reload for this sort of interaction instead, or if there's a nicer way to do "partial" Rust templates and manage those going back and forth between server and client? The code I can imagine using the current state of Rust web frameworks seems... not great, but perhaps my imagination is deficient :)

This would be another area where perhaps a bit more investigation/POC work would help convince me, if you're interested in doing more speculative work.

New user experience

I hear what you're saying with regards to the new user experience working on crates.io being overwhelming due to all the different technologies involved. My hope has actually been that ember would help bring in new contributors: JavaScript folks might see the Ember half of the app as something familiar, and they could work on that while slowly dipping into parts of the Rust app and easing into Rust that way. I think we have had a few people come in this path, but perhaps not enough to justify keeping things this way.

This is my weakest point, so it's last, but I did want to share this alternate perspective on why having the JavaScript part of the app might not necessarily be bad for new user experience. I'd love to have data on how many people were encouraged or discouraged from contributing due to crates.io's tech, but again, this will have to be weighed against the other factors above.

Thank you for your work @seanlinsley, and I'm up for continuing this conversation either here or in issues even though I'm going to close this ❤️ Thank you for your understanding.

@seanlinsley
Copy link
Author

I totally understand; like I said earlier I didn't expect this to be a quick transition.

I'm personally using this as an opportunity to learn Rust by working on a problem space (web apps) that I'm very familiar with. It's a good thing if that means adding features to get the tooling closer to production-quality.

Some things that would make me more confident would be:

  • Performance tests in a browser with realistic data, comparing rendering time between the current codebase and one using server-side rendering, showing a dramatic improvement
  • Full stack tests covering at least the happy path of all the major features of crates.io, of which we currently have none (we have front-end tests that mock the API and back-end tests that check the API responses, but no tests that go through both components from the browser)
  1. I could do that for the landing page, since that's already written. I haven't yet figured out how to get production-like data locally; any tips there would be helpful. Another option would be to write a detailed breakdown of the timing of current production page loads, showing where slowdowns could be eliminated.
  2. I'd be happy to work on this, though I could use some help understanding the current tests, and where they're lacking.

Re: interactivity, I've never logged in to crates.io, so I'm not sure exactly what Ember is doing to make the developer experience nicer.

Re: new contributor experience, I can certainly understand the idea that using an established JS framework might bring in contributors, but from my perspective the extra effort required to duplicate logic in both Rust and JS offsets the extra person-hours we're receiving.

I'm excited for the potential to run Rust isomorphically on both the server and the browser. While Yew appears to only render in the browser, it's nonetheless very promising CC @deniskolodin

@Turbo87
Copy link
Member

Turbo87 commented Jan 2, 2018

FWIW working on crates.io a few years ago got me into both rust and emberjs back then. I'm now mainly doing ember for my day job but I'm still interested in rust and play around with it from time to time :)

@nox
Copy link

nox commented Oct 15, 2018

@carols10cents It's been a year now that FastBoot support is "coming soon", maybe this RFC should be accepted after all? You say that crates.io works now, but #173 kinda disagrees.

@sgrif
Copy link
Contributor

sgrif commented Oct 16, 2018

@nox We're planning on discussing the possibility of switching to server side rendering soon, since the site is likely going to have a redesign happening.

@nox
Copy link

nox commented Oct 16, 2018

Could we decouple this discussion from any redesign debate? This is not about design or aesthetics, this is about letting visually-impaired people be able to use the website appropriately, this seems orders of magnitude more important than looks to me.

@ACleverDisguise
Copy link

This is because you, protestations to the contrary aside, care about actual end-users and not hypothetical ones who are design nerds.

@Turbo87
Copy link
Member

Turbo87 commented Oct 16, 2018

@nox moving to server-side rendering will not magically solve all a11y problems though. these things can also be solved in the current setup, we just need someone willing to invest time in solving them and it will be the same if we moved to SSR.

@nox
Copy link

nox commented Oct 16, 2018

@Turbo87 At no point I claimed it would solve all a11y problems, but it will definitely solve the problem that the website is entirely unusable without JS support, which is what people have been saying for 2 years now.

@Turbo87
Copy link
Member

Turbo87 commented Oct 16, 2018

it will definitely solve the problem that the website is entirely unusable without JS support

that seems entirely unrelated to "letting visually-impaired people be able to use the website appropriately" 🤔

@nox
Copy link

nox commented Oct 16, 2018

Did you even look at the issue I linked? From #173:

I'm blind and I use a braille display to access a computer. I would like to
use crates.io, since I've lately gotten interested about Rust. Unfortunately
the driver for my braille display (BRLTTY on GNU/Linux) only supports Linux
console, and therefore I have to use text-based web browsers (mostly elinks
and emacs-w3m). Neither of these browsers do support Ecmascript (JavaScript).
Crates.io relies heavily on JavaScript and therefore I cannot access the
package search and package information at all with my browsers.

@Turbo87
Copy link
Member

Turbo87 commented Oct 16, 2018

Indeed, I missed that part. I was under the impression that we were talking about screenreaders, which appears to be the thing that most visually-impaired people use or at least what most a11y guides usually focus on.

@sgrif
Copy link
Contributor

sgrif commented Oct 16, 2018

To be clear, the only reason this is being revisited as part of the redesign is that this is only feasible for us to take on as a feature if we're already going to be rewriting the HTML for another reason. That said, no decisions have been made. It's something we are planning on discussing.

@est31
Copy link
Member

est31 commented Oct 16, 2018

@sgrif there is no need to rewrite any HTML to get server side rendering going. The handlebars technology used by the current ember client side js is also usable by the server side Rust, via the awesome handlebars crate. This is precisely what this PR did: it used the existing hbs templates and evaluated them server side. This PR rewrote no HTML.

@seanlinsley
Copy link
Author

While most of the Handlebars code I used here was copy-pasted from the Ember folder structure, there were some modifications required (e.g. we were relying on helper functions).

I haven't followed the evolution of Rust web frameworks this year to know if they're at or near production quality yet. Assuming they're not, we could work around the potential security issues by only using server-side rendering for pages that are public and read-only.

@sunng87
Copy link
Contributor

sunng87 commented Oct 17, 2018 via email

@nox
Copy link

nox commented Aug 10, 2019

I know progress has been a long time coming on this, but the ember folks have told me we're only a few pull requests away from fastboot compatibility.

This was two damn years ago. Any news?

@est31
Copy link
Member

est31 commented Aug 10, 2019

@nox ember is on the way out. They are searching for volunteers for the redesign: rust-lang/crates-io-cargo-teams#29

TBH implementing the redesign seems like a good topic for a workgroup.

@jtgeibel
Copy link
Member

ember is on the way out

I would hardly say that Ember is on the way out. There are active efforts currently underway to enable fastboot, and in my opinion that is the most direct way toward achieving server-side rendering. Even if we already had team consensus on a path to eliminating the Ember frontend, I expect that we would be supporting the existing Ember code for at least a year or 2 during the transition.

I would personally like to first migrate away from the conduit-* crates and toward a framework that will help us handle concerns like CSRF, XSS, etc. We are the only reverse dependency on these crates, and I would prefer not to build such protections on top of our current backend and become further entrenched in this legacy design.

@carols10cents
Copy link
Member

This was two damn years ago. Any news?

@nox This hostile phrasing about what volunteers have or have not done is not appropriate here.

@nox ember is on the way out. They are searching for volunteers for the redesign: rust-lang/crates-io-cargo-teams#29

@est31 you are not on the team, please don't attempt to represent the team's positions.

@nox
Copy link

nox commented Aug 12, 2019

@nox This hostile phrasing about what volunteers have or have not done is not appropriate here.

This isn't about volunteers, this is about this RFC and server-side implementation being closed two years ago with the claim that fastboot support was very close to ready.

@carols10cents
Copy link
Member

carols10cents commented Aug 12, 2019

This isn't about volunteers, this is about this RFC and server-side implementation being closed two years ago with the claim that fastboot support was very close to ready.

The reason it hasn't become ready is because the volunteers that work on crates.io have been busy with other crates.io issues, other life issues, and finishing the fastboot support with problems that came up along the way after the comment was made, so yes, it is about volunteers. Your comment reads as you feel that you're entitled to volunteers having completed server-side rendering for you because 2 years have passed. If that's not what you intended, I suggest you try rephrasing.

@nox
Copy link

nox commented Aug 12, 2019 via email

@dwijnand
Copy link
Member

Even if you were entitled to that, would it have made any difference if this had stayed open but remained equally as unfinished as the fastboot support?

@seanlinsley
Copy link
Author

@nox this is how tickets end up getting locked by maintainers. Please, try to stay positive.

As @carols10cents previously stated, there are security features like CSRF and XSS protection that need to be in place before SSR could be realistically considered. Maybe in the past two years a Rust web framework has made progress on these?

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.

10 participants