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

Connect Middleware should return 404 if coordinates are out of bounds #31

Open
Mr0grog opened this issue Jan 29, 2013 · 10 comments
Open
Labels

Comments

@Mr0grog
Copy link
Member

Mr0grog commented Jan 29, 2013

I made a brief fix today so that our URL matching properly matches any series of three numbers (at deep enough zoom levels, you could easily have numbers that were too many digits to match).

However, we should probably actually check that the requested coordinates are in-bounds when projecting and return a 404 if they aren't.

(Note that the current situation is safe: we render an empty image and send that back if the coordinates are out of bounds. It would be better to be smart enough not to render anything in that case, though.)

@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 29, 2013

@yuletide Maybe pixelsToMeters, latLonToMeters, tileToMeters could return false if out of bounds? It looks like they each behave a bit differently in this regard (latLonToMeters clips to min/max extends, whereas pixelsToMeters just goes off and gives you an out-of-bounds number). Not sure what the other implications or issues with this might be.

Other options:

  • each of those functions could have a parameters indicating whether they should simply calculate, clip, or fail for out-of-bounds.
  • an extra inBounds(coordinate) method that returns true/false that the routes are expected to also call.

Thoughts?

@bensheldon
Copy link
Member

This would be a perfect opportunity to document interfaces in unit tests...

If we're going by return value, I would rather they return a new Error("coordinate out of bounds") object than false. (return, not throw).

I also will defer on what the implications are of passing back an out-of-bounds value.

@bensheldon
Copy link
Member

When we 404, should we return JSON with the specific error, or should we embed that in an image (big red 404 images with the error text overlaid?)

@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 29, 2013

Definitely NOT an image. Don't really care what the content we send back is otherwise.

@yuletide
Copy link
Member

We should investigate what other servers do first. Definitely not 404 images. But we don't want "missing image" placeholders to show up in the client either.

Alex Yule

On Tuesday, January 29, 2013 at 3:29 PM, Rob Brackett wrote:

Definitely NOT an image. Don't really care what the content we send back is otherwise.


Reply to this email directly or view it on GitHub (#31 (comment)).

@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 30, 2013

Well, a client that doing anything remotely reasonable shouldn't ever request such an image, right? If they're at a zoom level where there are 256x256 tiles, we're talking about the client asking for tile (256, 256), when it should be asking for tile (0, 256). I suppose we could also warp the results around so:

Request (256, 0) --> (0, 0)
Request (-1, 0) --> (255, 0)
Request (0, 256) --> (0, 0)
Request (0, -1) --> (0, 255)
etc.

@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 30, 2013

OSM returns a 404 with HTML and does not wrap around:
http://c.tile.openstreetmap.org/4/15/5.png (valid coordinate)
http://c.tile.openstreetmap.org/4/16/5.png (invalid coordinate)

@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 30, 2013

Mapbox, on the other hand, does what we are currently doing:
http://b.tiles.mapbox.com/v3/examples.map-40mxnz9l/4/15/5.png
http://b.tiles.mapbox.com/v3/examples.map-40mxnz9l/4/16/5.png

Not a 404 and also returns an image (just with nothing in it)

@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 30, 2013

Google returns a 400 (which actually makes more sense now that I think about it) rendered as HTML:
https://mts1.google.com/vt/lyrs=m@207000000&hl=en&src=app&x=15&y=5&z=4&scale=2&s=Galil
https://mts1.google.com/vt/lyrs=m@207000000&hl=en&src=app&x=16&y=5&z=4&scale=2&s=Galil

@Mr0grog
Copy link
Member Author

Mr0grog commented Jan 30, 2013

So:

  • Nobody wraps.
  • Some people return an error (400 or 404, rendered as HTML).
  • Some people return an image with nothing in it (what we are doing right now).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants