Skip to content

Prevent infinite recursion #5

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

Closed
wants to merge 1 commit into from
Closed

Prevent infinite recursion #5

wants to merge 1 commit into from

Conversation

zalmoxisus
Copy link
Contributor

@zalmoxisus zalmoxisus commented Aug 7, 2016

For example, ol3-react-example has a circular reference, which is not identified, because every reference has a new child. The object can be gotten with store.getState().

So it will be like (with i < 1000):

{geometry:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:87,U:{},A:[241942.11524857316,5069305.503773717,241942.11524857316,5069305.503773717],B:2,j:{},o:5982842.156102487,s:2,f:'XY',a:2,v:[241942.11524857316,5069305.503773717],olm_9971:{change:[{gg:{Fa:{},va:{'change:geometry':[null],change:[null],propertychange:[null]},g:1,closure_uid_730898495:88,U:{name:'Universitat Oberta de Catalunya',time:'2013-09-27'},a:'geometry',c:null,olm_9971:{'change:geometry':[{ig:false,type:'change:geometry'}],change:[{gg:{Fa:{},va:{change:[null]},g:1,closure_uid_730898495:1,U:{},f:null,xa:null,ya:'ready',N:true,na:{defaultDataProjection:{kb:'EPSG:4326',g:'degrees',c:[-180,-90,180,90],i:[-180,-90,180,90],b:'neu',f:true,a:true,j:null,l:111319.49079327358}},S:'OSGEoLabs.json',a:{a:{children:[{children:[{children:[[-6256558.025180174,-4151102.300370411,-6256558.025180174,-4151102.300370411,{Fa:{},va:{'change:geometry':[null],change:[null],propertychange:[null]},g:1,closure_uid_730898495:164,U:{geometry:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:163,U:{},A:[-6256558.025180174,-4151102.300370411,-6256558.025180174,-4151102.300370411],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-6256558.025180174,-4151102.300370411],olm_9971:{change:[{ig:false,type:'change'}]}},name:'Direccion Nacional de Topografia - MTOP',time:'2014-08-15'},a:'geometry',c:null,f:{ig:false,target:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:163,U:{},A:[-6256558.025180174,-4151102.300370411,-6256558.025180174,-4151102.300370411],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-6256558.025180174,-4151102.300370411],olm_9971:{change:[undefined]}},type:'change'},olm_9971:{'change:geometry':[{ig:false,type:'change:geometry'}],change:[{ig:false,type:'change'}],propertychange:[{ig:false,type:'propertychange'}]}}],[-6253532.806698376,-4150626.2699734597,-6253532.806698376,-4150626.2699734597,{Fa:{},va:{'change:geometry':[null],change:[null],propertychange:[null]},g:1,closure_uid_730898495:58,U:{geometry:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:57,U:{},A:[-6253532.806698376,-4150626.2699734597,-6253532.806698376,-4150626.2699734597],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-6253532.806698376,-4150626.2699734597],olm_9971:{change:[{ig:false,type:'change'}]}},name:'<a href="http://www.universidad.edu.uy/" class="external text" rel="nofollow" target="_blank">Universidad de la República</a>',time:'2013-03-01'},a:'geometry',c:null,f:{ig:false,target:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:57,U:{},A:[-6253532.806698376,-4150626.2699734597,-6253532.806698376,-4150626.2699734597],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-6253532.806698376,-4150626.2699734597],olm_9971:{change:[undefined]}},type:'change'},olm_9971:{'change:geometry':[{ig:false,type:'change:geometry'}],change:[{ig:false,type:'change'}],propertychange:[{ig:false,type:'propertychange'}]}}],[-7586946.499168323,-1862106.7671920976,-7586946.499168323,-1862106.7671920976,{Fa:{},va:{'change:geometry':[null],change:[null],propertychange:[null]},g:1,closure_uid_730898495:116,U:{geometry:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:115,U:{},A:[-7586946.499168323,-1862106.7671920976,-7586946.499168323,-1862106.7671920976],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-7586946.499168323,-1862106.7671920976],olm_9971:{change:[{ig:false,type:'change'}]}},name:'<a href="http://www.geo.gob.bo/" class="external text" rel="nofollow" target="_blank">GeoBolivia-Vice-presidency of the State </a>',time:'2013-11-24'},a:'geometry',c:null,f:{ig:false,target:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:115,U:{},A:[-7586946.499168323,-1862106.7671920976,-7586946.499168323,-1862106.7671920976],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-7586946.499168323,-1862106.7671920976],olm_9971:{change:[undefined]}},type:'change'},olm_9971:{'change:geometry':[{ig:false,type:'change:geometry'}],change:[{ig:false,type:'change'}],propertychange:[{ig:false,type:'propertychange'}]}}],[-7627768.2469981415,-3701416.5741028963,-7627768.2469981415,-3701416.5741028963,{Fa:{},va:{'change:geometry':[null],change:[null],propertychange:[null]},g:1,closure_uid_730898495:36,U:{geometry:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:35,U:{},A:[-7627768.2469981415,-3701416.5741028963,-7627768.2469981415,-3701416.5741028963],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-7627768.2469981415,-3701416.5741028963],olm_9971:{change:[{ig:false,type:'change'}]}},name:'National University of San Juan',time:'2012-06-15'},a:'geometry',c:null,f:{ig:false,target:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:35,U:{},A:[-7627768.2469981415,-3701416.5741028963,-7627768.2469981415,-3701416.5741028963],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-7627768.2469981415,-3701416.5741028963],olm_9971:{change:[undefined]}},type:'change'},olm_9971:{'change:geometry':[{ig:false,type:'change:geometry'}],change:[{ig:false,type:'change'}],propertychange:[{ig:false,type:'propertychange'}]}}],[-8153683.93423156,-417282.92206393497,-8153683.93423156,-417282.92206393497,{Fa:{},va:{'change:geometry':[null],change:[null],propertychange:[null]},g:1,closure_uid_730898495:52,U:{geometry:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:51,U:{},A:[-8153683.93423156,-417282.92206393497,-8153683.93423156,-417282.92206393497],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-8153683.93423156,-417282.92206393497],olm_9971:{change:[{ig:false,type:'change'}]}},name:'National Amazonian University of Madre de Dios (UNAMAD)',time:'2013-02-01'},a:'geometry',c:null,f:{ig:false,target:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:51,U:{},A:[-8153683.93423156,-417282.92206393497,-8153683.93423156,-417282.92206393497],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-8153683.93423156,-417282.92206393497],olm_9971:{change:[undefined]}},type:'change'},olm_9971:{'change:geometry':[{ig:false,type:'change:geometry'}],change:[{ig:false,type:'change'}],propertychange:[{ig:false,type:'propertychange'}]}}],[-11419053.17803229,2603116.960874135,-11419053.17803229,2603116.960874135,{Fa:{},va:{'change:geometry':[null],change:[null],propertychange:[null]},g:1,closure_uid_730898495:64,U:{geometry:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:63,U:{},A:[-11419053.17803229,2603116.960874135,-11419053.17803229,2603116.960874135],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-11419053.17803229,2603116.960874135],olm_9971:{change:[{ig:false,type:'change'}]}},name:'<a href="http://www.cozcyt.gob.mx/labsol/" class="external text" rel="nofollow" target="_blank">Laboratorio de Software Libre</a>',time:'2013-04-05'},a:'geometry',c:null,f:{ig:false,target:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:63,U:{},A:[-11419053.17803229,2603116.960874135,-11419053.17803229,2603116.960874135],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-11419053.17803229,2603116.960874135],olm_9971:{change:[undefined]}},type:'change'},olm_9971:{'change:geometry':[{ig:false,type:'change:geometry'}],change:[{ig:false,type:'change'}],propertychange:[{ig:false,type:'propertychange'}]}}]],height:1,bbox:[-11419053.17803229,-4151102.300370411,-6253532.806698376,2603116.960874135],La:true},{children:[[-10834592.344200876,3213026.9771947945,-10834592.344200876,3213026.9771947945,{Fa:{},va:{'change:geometry':[null],change:[null],propertychange:[null]},g:1,closure_uid_730898495:78,U:{geometry:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:77,U:{},A:[-10834592.344200876,3213026.9771947945,-10834592.344200876,3213026.9771947945],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-10834592.344200876,3213026.9771947945],olm_9971:{change:[{ig:false,type:'change'}]}},name:'<a href="http://spatialquerylab.com/projects/open-source-gis/" class="external text" rel="nofollow" target="_blank">Spatial {Query} Lab at Texas A&amp;M University - Corpus Christi</a>',time:'2013-09-09'},a:'geometry',c:null,f:{ig:false,target:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:77,U:{},A:[-10834592.344200876,3213026.9771947945,-10834592.344200876,3213026.9771947945],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-10834592.344200876,3213026.9771947945],olm_9971:{change:[undefined]}},type:'change'},olm_9971:{'change:geometry':[{ig:false,type:'change:geometry'}],change:[{ig:false,type:'change'}],propertychange:[{ig:false,type:'propertychange'}]}}],[-10843308.326371515,3219258.7074819384,-10843308.326371515,3219258.7074819384,{Fa:{},va:{'change:geometry':[null],change:[null],propertychange:[null]},g:1,closure_uid_730898495:136,U:{geometry:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:135,U:{},A:[-10843308.326371515,3219258.7074819384,-10843308.326371515,3219258.7074819384],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-10843308.326371515,3219258.7074819384],olm_9971:{change:[{ig:false,type:'change'}]}},name:'<a href="http://foss4geo.org" class="external text" rel="nofollow" target="_blank">FOSS4Geo Academy @ Del Mar College National Open Geospatial Technology Consortium</a>',time:'2014-02-18'},a:'geometry',c:null,f:{ig:false,target:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:135,U:{},A:[-10843308.326371515,3219258.7074819384,-10843308.326371515,3219258.7074819384],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-10843308.326371515,3219258.7074819384],olm_9971:{change:[undefined]}},type:'change'},olm_9971:{'change:geometry':[{ig:false,type:'change:geometry'}],change:[{ig:false,type:'change'}],propertychange:[{ig:false,type:'propertychange'}]}}],[-12461104.355996497,3951920.4568047756,-12461104.355996497,3951920.4568047756,{Fa:{},va:{'change:geometry':[null],change:[null],propertychange:[null]},g:1,closure_uid_730898495:86,U:{geometry:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:85,U:{},A:[-12461104.355996497,3951920.4568047756,-12461104.355996497,3951920.4568047756],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-12461104.355996497,3951920.4568047756],olm_9971:{change:[{ig:false,type:'change'}]}},name:'<a href="http://geodacenter.asu.edu" class="external text" rel="nofollow" target="_blank">GeoDa Center for Geospatial Analysis and Computation, Arizona State University</a>',time:'2013-09-27'},a:'geometry',c:null,f:{ig:false,target:{Fa:{},va:{change:[null]},g:2,closure_uid_730898495:85,U:{},A:[-12461104.355996497,3951920.4568047756,-12461104.355996497,3951920.4568047756],B:2,j:{},o:0,s:0,f:'XY',a:2,v:[-12461104.355996497,3951920.4568047756],olm_9971:{change:[undefined]}},type:'change'},olm_9971:{'change:geometry':[{ig:false,type:'change:geometry'}],change:[{ig:false,type:'change'}],propertychange:[{ig:false}]}}],undefined,undefined,undefined]},undefined,undefined,undefined,undefined,undefined,undefined,undefined]},undefined]}}}}]}}}]}}}

You can copy-paste it to the console to see the structure.

As the result, the recursion is infinite and hang the browser. A simple solution is to have a limit set to 100.

@zalmoxisus zalmoxisus changed the title Prevent infinite recurtion Prevent infinite recursion Aug 7, 2016
@coveralls
Copy link

coveralls commented Aug 7, 2016

Coverage Status

Coverage increased (+0.05%) to 96.203% when pulling 5be7a95 on zalmoxisus:patch-2 into 4da13e4 on blakeembrey:master.

@coveralls
Copy link

coveralls commented Aug 7, 2016

Coverage Status

Coverage increased (+0.05%) to 96.203% when pulling 2fa1c38 on zalmoxisus:patch-2 into 4da13e4 on blakeembrey:master.

@blakeembrey
Copy link
Owner

@zalmoxisus Do you want to make it a maxDepth option? Not sure how useful it is, since you should probably throw instead of just stopping randomly.

The other issue is i++ is currently based on the total items, not the depth. It'd be easier to start at 0 and pass it along + 1 with each function call (like the cache is used).

@zalmoxisus
Copy link
Contributor Author

zalmoxisus commented Aug 7, 2016

@blakeembrey, throwing would break the app, but stopping will just not show nested objects in depth, which is not a problem for circular references. So, we could use it for those cases without problems.

Not sure, I understand the suggestion about maxDepth option, as I don't see how we can pass options in the current API. Though one could want more iterations than 100, so having it configurable makes sense.

Of course, better to find a solution to catch such cases and ignore as other circular references, but since I don't see one, am coming with this workaround :)

@blakeembrey
Copy link
Owner

I think adding a new options object at the end seems reasonable. I was considering doing such a thing in the future to support the idea of restoring circular references too. On throwing, it's fine not too - I just wasn't fully understanding the use-case and figured from a programatic point of view it's better to throw, but from your I'm guessing it's all about the rendering so it's not an issue?

Just remember to patch the i issue, or I can tomorrow if you let me know you can't 😄

@zalmoxisus
Copy link
Contributor Author

@blakeembrey, ok, thx for the details. I made the changes. Also added the tests and updated the README. Please amend it when you get time.

@coveralls
Copy link

coveralls commented Aug 7, 2016

Coverage Status

Coverage increased (+0.3%) to 96.429% when pulling a17dc5a on zalmoxisus:patch-2 into 4da13e4 on blakeembrey:master.

@blakeembrey
Copy link
Owner

Merged with 8401708, cheers!

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.

3 participants