jeff-zucker / data-kitchen

The solid databrowser technology as a stand-alone electron app
4 stars 2 forks source link

source in filemanager should be url encoded #5

Closed bourgeoa closed 4 years ago

bourgeoa commented 4 years ago

A check should be made to verify that source is url encoded in kitchen.js No need for destination.

Is it only for filemanager or should it go in mungeURI ?

jeff-zucker commented 4 years ago

If the source comes from the data kitchen it should already be encoded. Is it possible to check if a URI has already been encoded? We don't want to double-encode.

jeff-zucker commented 4 years ago

I don't believe we need to do anything extra in file manager.

bourgeoa commented 4 years ago

this function should do the trick :

var getUrlEncoded = sURL => { if (decodeURI(sURL) === sURL) return encodeURI(sURL) return getUrlEncoded(decodeURI(sURL)) }

I think I used it in solid-ide

jeff-zucker commented 4 years ago

Yes, but why would we need it? URIs that need encoding that aren't encoded will break the databrowser before they get to the filemanager.

bourgeoa commented 4 years ago

Just for when you type the source manually knowing the name of the file. You don't know the URL encoded one.

jeff-zucker commented 4 years ago

OK ,as a convenience, yes, sounds good. Should go in mungeURI I guess. What is getUrlEncoded() ?

bourgeoa commented 4 years ago

not written by me taken from https://stackoverflow.com/questions/2295223/how-to-find-out-if-string-has-already-been-url-encoded (last comments)

jeff-zucker commented 4 years ago

This fails badly with file names that have a plus sign (+) in them. So I am not going to implement this.

bourgeoa commented 4 years ago

I went back to NSS and in fact they do not use encodeURI() but :

const url = ${this.resolveUrl(hostname)}${ pathname.split(pathUtil.sep).map((component) => encodeURIComponent(component)).join('/') }

This encode url function should not fail with +

jeff-zucker commented 4 years ago

It seems to me, that the safest thing to do is check to see if there are any forbidden characters in the URI. If there are, encode it, otherwise, do nothing. But that looks like a good way to do the encoding.

jeff-zucker commented 4 years ago

The nodejs URL object does the right thing in all circumstances. Checkout what it does with a URL that is partially encoded and partially not and even has a missing slash on the protocol (outputs "true:):

const bad  = "https:/example.com/foo bar%20baz+bop"
const good = 'https://example.com/foo%20bar%20baz+bop'
try {
  let url = new URL (bad)
  console.log(url.href===good)
}
catch(e){ console.log(e.code) }
jeff-zucker commented 4 years ago

Yep, this works, added to mungeURI