Opening windows with large configurations results in a 404

[bfattori]

Currently you appear to be passing the entire configuration for a pop-out via the URL. When a configuration/componentState is especially large, the result is a 404. I have a suggestion to make this work, and not completely compromise the platform for XSS style attacks.

Instead of sending the entire configuration, send a configuration Id of some sort. Then, when the window initializes it would use cross-window messaging to "request" its configuration. I haven't tried this yet, but intend to... Unless you feel this could be a quick fix.

[bfattori]

Just so you know what I'm talking about, large would be like:

componentState: {
    directive: 'watch-list',
        symbols: ['ABT', 'ABBV', 'ACE', 'ACN', 'ACT', 'ADBE', 'ADT', 'AES', 'AET', 'AFL', 'AMG', 'A', 'GAS', 'APD', 'ARG', 'AKAM', 'AA',
        'ALXN', 'ATI', 'ALLE', 'AGN', 'ADS', 'ALL', 'ALTR', 'MO', 'AMZN', 'AEE', 'AEP', 'AXP', 'AIG', 'AMT', 'AMP', 'ABC', 'AME',
        'AMGN', 'APH', 'APC', 'ADI', 'AON', 'APA', 'AIV', 'AAPL', 'AMAT', 'ADM', 'AIZ', 'T', 'ADSK', 'ADP', 'AN', 'AZO', 'AVGO',
        'AVB', 'AVY', 'AVP', 'BHI', 'BLL', 'BAC', 'BK', 'BCR', 'BAX', 'BBT', 'BDX', 'BBBY', 'BMS', 'BRK.B', 'BBY', 'BIIB', 'BLK',
        'HRB', 'BA', 'BWA', 'BXP', 'BSX', 'BMY', 'BRCM', 'BF.B', 'CHRW', 'CA', 'CVC', 'COG', 'CAM', 'CPB', 'COF', 'CAH', 'CFN',
        'KMX', 'CCL', 'CAT', 'CBG', 'CBS', 'CELG', 'CNP', 'CTL', 'CERN', 'CF', 'SCHW', 'CHK', 'CVX', 'CMG', 'CB', 'CI', 'XEC',
        'CINF', 'CTAS', 'CSCO', 'C', 'CTXS', 'CLX', 'CME', 'CMS', 'COH', 'KO', 'CCE', 'CTSH', 'CL', 'CMCSA', 'CMA', 'CSC', 'CAG',
        'COP', 'CNX', 'ED', 'STZ', 'GLW', 'COST', 'COV', 'CCI', 'CSX', 'CMI', 'CVS', 'DHI', 'DHR', 'DRI', 'DVA', 'DE', 'DLPH',
        'DAL', 'DNR', 'XRAY', 'DVN', 'DO', 'DTV', 'DFS', 'DISCA', 'DISCK', 'DG', 'DLTR', 'D', 'DOV', 'DOW', 'DPS', 'DTE', 'DD',
        'DUK', 'DNB', 'ETFC', 'EMN', 'ETN', 'EBAY', 'ECL', 'EIX', 'EW', 'EA', 'EMC', 'EMR', 'ESV', 'ETR', 'EOG', 'EQT', 'EFX',
        'EQR', 'ESS', 'EL', 'EXC', 'EXPE', 'EXPD', 'ESRX', 'XOM', 'FFIV', 'FB', 'FDO', 'FAST', 'FDX', 'FIS', 'FITB', 'FSLR', 'FE',
        'FISV', 'FLIR', 'FLS', 'FLR', 'FMC', 'FTI', 'F', 'FOSL', 'BEN', 'FCX', 'FTR', 'GME', 'GCI', 'GPS', 'GRMN', 'GD', 'GE',
        'GGP', 'GIS', 'GM', 'GPC', 'GNW', 'GILD', 'GS', 'GT', 'GOOGL', 'GOOG', 'GWW', 'HAL', 'HOG', 'HAR', 'HRS', 'HIG', 'HAS',
        'HCP', 'HCN', 'HP', 'HES', 'HPQ', 'HD', 'HON', 'HRL', 'HSP', 'HST', 'HCBK', 'HUM', 'HBAN', 'ITW', 'IR', 'TEG', 'INTC',
        'ICE', 'IBM', 'IP', 'IPG', 'IFF', 'INTU', 'ISRG', 'IVZ', 'IRM', 'JEC', 'JNJ', 'JCI', 'JOY', 'JPM', 'JNPR', 'KSU', 'K',
        'KEY', 'GMCR', 'KMB', 'KIM', 'KMI', 'KLAC', 'KSS', 'KRFT', 'KR', 'LB', 'LLL', 'LH', 'LRCX', 'LM', 'LEG', 'LEN', 'LVLT',
        'LUK', 'LLY', 'LNC', 'LLTC', 'LMT', 'L', 'LO', 'LOW', 'LYB', 'MTB', 'MAC', 'M', 'MMM', 'MNK', 'MRO', 'MPC', 'MAR', 'MMC',
        'MAS', 'MA', 'MAT', 'MKC', 'MCD', 'MHFI', 'MCK', 'MJN', 'MWV', 'MDT', 'MRK', 'MET', 'MCHP', 'MU', 'MSFT', 'MHK', 'TAP',
        'MDLZ', 'MON', 'MNST', 'MCO', 'MS', 'MOS', 'MSI', 'MUR', 'MYL', 'NBR', 'NDAQ', 'NOV', 'NAVI', 'NTAP', 'NFLX', 'NWL', 'NFX',
        'NEM', 'NWSA', 'NEE', 'NLSN', 'NKE', 'NI', 'NE', 'NBL', 'JWN', 'NSC', 'NTRS', 'NOC', 'NU', 'NRG', 'NUE', 'NVDA', 'KORS',
        'ORLY', 'OXY', 'OMC', 'OKE', 'ORCL', 'OI', 'PCG', 'PCAR', 'PLL', 'PH', 'PDCO', 'PAYX', 'PNR', 'PBCT', 'POM', 'PEP', 'PKI',
        'PRGO', 'PETM', 'PFE', 'PM', 'PSX', 'PNW', 'PXD', 'PBI', 'PCL', 'PNC', 'RL', 'PPG', 'PPL', 'PX', 'PCP', 'PCLN', 'PFG', 'PG',
        'PGR', 'PLD', 'PRU', 'PEG', 'PSA', 'PHM', 'PVH', 'QEP', 'PWR', 'QCOM', 'DGX', 'RRC', 'RTN', 'RHT', 'REGN', 'RF', 'RSG',
        'RAI', 'RHI', 'ROK', 'COL', 'ROP', 'ROST', 'R', 'SWY', 'CRM', 'SNDK', 'SCG', 'SLB', 'SNI', 'STX', 'SEE', 'SRE', 'SHW', 'SIAL',
        'SPG', 'SJM', 'SNA', 'SO', 'LUV', 'SWN', 'SE', 'STJ', 'SWK', 'SPLS', 'SBUX', 'HOT', 'STT', 'SRCL', 'SYK', 'STI', 'SYMC',
        'SYY', 'TROW', 'TGT', 'TEL', 'TE', 'THC', 'TDC', 'TSO', 'TXN', 'TXT', 'HSY', 'TRV', 'TMO', 'TIF', 'TWX', 'TWC', 'TJX',
        'TMK', 'TSS', 'TSCO', 'RIG', 'TRIP', 'FOXA', 'TSN', 'TYC', 'USB', 'UA', 'UNP', 'UNH', 'UPS', 'MLM', 'URI', 'UTX', 'UHS',
        'UNM', 'URBN', 'VFC', 'VLO', 'VAR', 'VTR', 'VRSN', 'VZ', 'VRTX', 'VIAB', 'V', 'VNO', 'VMC', 'WMT', 'WAG', 'DIS', 'WM',
        'WAT', 'WLP', 'WFC', 'WDC', 'WU', 'WY', 'WHR', 'WFM', 'WMB', 'WIN', 'WEC', 'WYN', 'WYNN', 'XEL', 'XRX', 'XLNX', 'XL',
        'XYL', 'YHOO', 'YUM', 'ZMH', 'ZION', 'ZTS']
}

That's the entire S&P 500 set of stock symbols... Obviously I could code for this specific case, but the application allows for the open-ended configuration of whatever symbols the user wants. For example, there's the Russell 2000 which contains (yep, you guessed it) 2000 symbols.

[deepstreamIO]

Hi,

it's true, browsers impose a 4096 character limit - hence the minification step as well. It's a bit of a trade-off. Originally the configuration was just transferred via the window.opener - which fundamentally works well, but in practice has a number of edge conditions ( IE passing the config by reference whereas every other browser passes it by copy for instance).

The main reason we went for the URL parameter in the end is that we wanted to avoid having a parent-child relationship between the windows. With the current setup every window is truly independent and can be opened and closed without affecting the others. And the event-hub between the windows facilitates communication and keeps things loosely coupled.

To be honest, I think that there's a lot of value in having this kind of independence - but I also complete understand that that doesn't help much with your usecase. I'll look into a solution for the next release - I'd like to keep communication between the windows to a minimum - due to the reasons explained above - but other strategies, e.g. writing and retrieving config from localStorage might be a viable alternativ that could be supported as an optional configuration parameters. Let me know your thoughts, would this work for you?

[bfattori]

I like the idea of using localStorage to pass the configuration. But it won't work, as it currently is for us, by sending it on the URL. Golden Layout is pretty much a shoe-in for our project but this is one issue that we'd need to get corrected (or correct, ourselves).

I'm always a fan of loose coupling, but not when it makes things unusable. As far as I can be concerned on this, a large configuration will break the functionality (which is so unique to GL, btw) we need.

[deepstreamIO]

Sorry, what I meant is: I'd be happy to add a configuration parameter to the next release, e.g.

transferStateViaLocalStorage: true

which would write the component's state to localStorage and only add an id to the url parameter - the new popout window would than use this identifier to retrieve its state from localStorage - they are after all under the same domain .

However, in order to resolve your current problem you would need to change your http server's configuration, rather than GoldenLayout. Your server is the one that returns the 404 - your browser is perfectly happy to handle longer URLs as can be demonstrated by visiting this link with >5000 characters (hosted on amazon cloud front)

[bfattori]

Yeah, browsers do not have a limitation, but setting the amount of data to be passed on the URL much higher than 4k is starting to get crazy. There are much better ways to pass the data, so I like the idea of the configuration parameter to pass via local storage.

[scottmunday84]

Probably the best way around it would be to defer the componentState's retrieval to within the component itself. That way, you can retrieve data, throttle it, whatever upon load, and not on setup.

[deepstreamIO]

Thanks again for pointing this out. Version 1.0.6 was just released which now passes configuration through localStorage. The only thing that's appended to the URL is a key that points to the localStorage item. This gives you 5MB of configuration to use - as well as closes a number of XSS attack vectors that resulted from retrieving data from the URL.

[bfattori]

One thing that I've been running into is opening these windows and getting
a race condition. It's storing some variables on an object for the new pop
out, I see. Then some methods execute which need
BrowserPopout.prototype.getGlInstance(), however it's not initialized yet
in the window (or so it seems).

On Sun, Dec 14, 2014 at 5:21 AM, hoxton-one notifications@github.com
wrote:

Thanks again for pointing this out. Version 1.0.6 was just released which
now passes configuration through localStorage. The only thing that's
appended to the URL is a key that points to the localStorage item. This
gives you 5MB of configuration to use - as well as closes a number of XSS
attack vectors that resulted from retrieving data from the URL.

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

[deepstreamIO]

this is worrying, thanks for letting me know. I've tested the localStorage feature quite extensively and haven't seen it yet. Could you let me know which browser you're using and if there's anything specific about your implementation?

[bmuenzenmeyer]

Curious, would you expect this convention to preserve the state of the content within a popout?

Use case: I start editing a form, only to pop it out to continue filling it out elsewhere.

In my experience thus far, popping out the window reinitializes the form, discarding unsaved data.

[deepstreamIO]

I'm afraid that will still be the case. Most attempts to create multi window web-apps copy dom nodes to the new window - which seems to be the most obvious approach, but comes with a whole world of edge conditions: global js objects not being present,link tags that need to be copied individually, IE implicitly referencing objects in the parent window etc...

Creating a new component with the same state has proven to be a way more reliable approach - it does however mean that you'd need to keep said state permanently up to date. Fortunately most MV* frameworks make this quite simple...you could for instance just call setState() on every keypress and save your serialized Angular / Backbone / KO model - whilst at the same time only saving the actual layout state to the backend after a couple of seconds.

[wiscow]

Following up on @bmuenzenmeyer question. Is there a way to detect when a constainer is being flown out into a popout? I notice that there is the windowOpened event that fires when a popout occurs. However, there is no event that takes previous to the window opening. It would be nice if there was an event on the container that was fired prior to opening that container in a popout. This way you could save the state without constantly having to write to local storage.

[deepstreamIO]

Hi @wiscow ,

I'm afraid there currently isn't a way to be notified before a popout is created, but I feel that you've got a very valid use-case and will add a 'beforePopout' event for the next release.

In the meantime you could easily monkey-patch it as follows:

(function(){
    var _createPopout = GoldenLayout.prototype.createPopout;

    GoldenLayout.prototype.createPopout = function( contentItem ) {
        this.emit( 'beforePopout', contentItem );
        _createPopout.apply( this, Array.prototype.slice.call(arguments) );
    };
})();

[wiscow]

Hey @hoxton-one ,

Thanks for the response. Currently, i am kind of bastardizing 'destroy' in order to achieve something similar. Also, I was digging through the source and found a 'PopIn' event on the layout that was not documented. I was able to use this for saving when the window is reconstituted in the main layout. Just wanted to give you a heads up on that since it is not currently documented on your site.

Kyle

[deepstreamIO]

Thanks for letting me know... sometimes even I am surprised by just how
many features this thing actually has :-)

2015-01-26 17:15 GMT+00:00 Kyle MacDonald notifications@github.com:

Hey @hoxton-one https://github.com/hoxton-one ,

Thanks for the response. Currently, i am kind of bastardizing 'destroy' in
order to achieve something similar. Also, I was digging through the source
and found a 'PopIn' event on the layout that was not documented. I was able
to use this for saving when the window is reconstituted in the main layout.
Just wanted to give you a heads up on that since it is not currently
documented on your site.

Kyle

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