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

Deep POST params versus numeric index parsing #33

Open
ztmr opened this issue Jan 7, 2014 · 4 comments
Open

Deep POST params versus numeric index parsing #33

ztmr opened this issue Jan 7, 2014 · 4 comments

Comments

@ztmr
Copy link

ztmr commented Jan 7, 2014

Deep POST params parsing does not work properly with numeric indices:

> (simple_bridge_request_wrapper:new ([], [], [], [], [], [])):parse_deep_post_params ([{"x[3]","x3:val"}], []).
[{"x",[2,1,0,"x3:val"]}]

...but if we make the index an alphanumeric word, it starts to behave as expected:

> (simple_bridge_request_wrapper:new ([], [], [], [], [], [])):parse_deep_post_params ([{"x[i3]","x3:val"}], []).
[{"x",[{"i3","x3:val"}]}]
@choptastic
Copy link
Member

Hi, Thanks for the find. This is definitely an inconsistency that will need fixing. Indeed, I suspect treating numeric indices as numbers (ala PHP) probably shouldn't be done at all. That said, these changes were brought in to accommodate ChicagoBoss, so I wouldn't want to make such a significant change without first ensuring such a change wouldn't break anything.

@ztmr
Copy link
Author

ztmr commented Jan 8, 2014

No problem, I just had to make a quick'n'dirty hack in ztmr/simple_bridge@d767320 because it blocked me in preparing a new version of our application that was going to be deployed on customer's site today.
So it is in production now, but I hope this hack won't be temporary forever :-)

@choptastic
Copy link
Member

Hi Tomas,

I was checking this out again, as I'm re-entering a stage where I plan on finishing up SimpleBridge 2.0, and I actually quite like this solution.

That said, how has this change been holding up in your production systems? Do you ruh this with ChicagoBoss, I assume?

@ztmr
Copy link
Author

ztmr commented Mar 20, 2014

Both of the patches (ztmr/simple_bridge@d767320 and ztmr/simple_bridge@a101814) worked as a charm, so if you're happy with the changes, let's merge them and be happy all together! ;-)

And yes, we use SB with a little older and hacked version of CB. As well as we need to stay with Erlang 15 for multiple reasons at the moment :(

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

No branches or pull requests

2 participants