search

Financial-Times / s3o-middleware

Middleware to handle authenticating with S3O
3 stars 9 forks source link

Drops some query strings #15

Closed wheresrhys closed 7 years ago

wheresrhys commented 8 years ago

e.g. http://ft-next-beacon-v2.herokuapp.com/data/query-wizard?query=@concat(cta:click-%3Ecount()-%3Efilter(context.domPath~follow-promo-header),cta:click-%3Ecount()-%3Efilter(context.domPath~suggested-article),cta:click-%3Ecount()-%3Efilter(context.domPath~section-link),cta:click-%3Ecount()-%3Efilter(context.domPath~next-up-header),)-%3Efilter(page.location.type=article)-%3ErelTime(1)

wheresrhys commented 8 years ago

Initial investigation suggests it doesn't like it when they begin with an @

matthew-andrews commented 8 years ago

i think those should probably be better URLs if the @ were encodeURIComponented => %40.

wheresrhys commented 8 years ago

yep, will definitely be doing that, but thought I'd report as maybe s3o should attempt to handle this scenario. There are lots of other non encoded characters in the url that it does cope with, so its easy for people to think non-uri encoding is ok, but then get caught out by this.

JakeChampion commented 7 years ago

I don't think S3O should deal with this, it is a simple piece of middleware that does one job, ExpressJS handles the URL parsing for us.