Marco Bonardo [:mak] Assignee
	Description • 14 years ago

In Bug 563738 i'm adding contents to about:home using localStorage. Chrome context writes and content context reads data, since this page is going to contain a mix of local and remote content we felt like makes sense to increase its security.

To "pipe" the data I need localStorage to be able to store data for about: pages, these have a scheme and a path, no host.

I will start proposing using the path for URIs with empty hosts, gather feedback, then finalize what I get with a test.

	Marco Bonardo [:mak] Assignee
	Comment 2 • 14 years ago

I just noticed Bug 357323 that looks similar and has some insight, will read through it.
Notice about: pages are really simple and have only scheme and path that are more than enough to identify each one of them. So maybe I could restrict my solution to about: scheme.
Btw, reading.

	Marco Bonardo [:mak] Assignee
	Comment 3 • 14 years ago

Hm, reading through that bug looks like specs are vague about that, and the greates issue is with file:// acces...
actually would it be problem if only for about: pages I'd use ascii path as domain? they are served by our redirector, there should be no risk of another external page looking into their content (unless I miss something in the middle).

	Honza Bambas (:mayhemer)
	Comment 4 • 14 years ago

Comment on attachment 447386 [details] [diff] [review]
patch v1.0

Would be great to limit this to about: scheme only, the question of files has not been solved so far.  Then, would be good to append a suffix to the path that would protect the scope from potentially overlapping with other TLDs and would keep the system consistent.  Suffixes to select from could be ".about" or better ".about.mozilla" or something like that.

	Marco Bonardo [:mak] Assignee
	Comment 5 • 14 years ago

Taking a look at the db I've noticed this is saved as "emoh.:moz-safe-about"
So if I read correctly your suggestion is to have "emoh.about.mozilla:moz-safe-about"? I thought the part after the colon was already there to protect against that kind of overlapping

	Marco Bonardo [:mak] Assignee
	Comment 6 • 14 years ago

first try at what you suggested, with a small unit test.

	Honza Bambas (:mayhemer)
	Comment 7 • 14 years ago

(In reply to comment #5)
> Taking a look at the db I've noticed this is saved as "emoh.:moz-safe-about"
> So if I read correctly your suggestion is to have
> "emoh.about.mozilla:moz-safe-about"? I thought the part after the colon was
> already there to protect against that kind of overlapping

You are absolutely right.  I completely forgot that for localStorage we used the OriginScopeDBKey that also includes the schema.  Sorry for bad advise with the suffix.  Your patch then just needed to have an artificial limitation to work just for the 'about:' schema.

You can probably use aURI->SchemeIs("about") to check.

I don't think that in case of applying this just to the about: protocol needs to use IDN conversion.  If you see a reason to, feel free to oppose.  BTW - another case to add asciiPath on nsIURI...

	Marco Bonardo [:mak] Assignee
	Comment 8 • 14 years ago

Yeah, effectively I don't expect us to use any idn feature internally, thus using the path as it is should be fine. Also removed other redundant code based on your comment.

	Marco Bonardo [:mak] Assignee
	Comment 9 • 14 years ago

Comment on attachment 449225 [details] [diff] [review]
patch v3.0

gah, this should also handle moz-safe-about scheme

	Marco Bonardo [:mak] Assignee
	Comment 10 • 14 years ago

Here it is, but there is something strange, the test was passing also without the change, and it was not complaining about being unable to create a persistent database (i missed do_get_profile();) there must be code handling this case and falling back to memory hash

	Honza Bambas (:mayhemer)
	Comment 11 • 14 years ago

(In reply to comment #10)
> Here it is, but there is something strange, the test was passing also without
> the change, and it was not complaining about being unable to create a
> persistent database (i missed do_get_profile();) there must be code handling
> this case and falling back to memory hash

It is: http://mxr.mozilla.org/mozilla-central/source/dom/src/storage/nsDOMStorage.cpp#665

W/o "the change" the scope key is empty (fails to create) so, the storage will use just the in-mem db.  You can see NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file d:/mozilla/mozilla-central/dom/src/storage/nsDOMStorageDBWrapper.cpp, line 324 in the console, the place the scope key is being created.


...would be cool to preserve the profile after the test and run a second test (after shell restart) to check the value is still there, but xpc-shell test doesn't provide (AFAIK) such functionality.

	Honza Bambas (:mayhemer)
	Comment 12 • 14 years ago

Comment on attachment 449447 [details] [diff] [review]
patch v3.1

r=me

	Marco Bonardo [:mak] Assignee
	Comment 13 • 14 years ago

thanks
http://hg.mozilla.org/mozilla-central/rev/60a14de5241c