cappy icon indicating copy to clipboard operation
cappy copied to clipboard

Windows fixes

Open laerreal opened this issue 5 years ago • 6 comments

Hello! Ricently I used this tool under Windows 7 x64, Python 2.7.15 x86_64. There are few fixes I made.

Using OS specific separator instead of : will cause misses on old caches. But this does make new caches portable.

laerreal avatar Jan 06 '19 14:01 laerreal

Hi @laerreal

Welcome to Cappy and thank you for the PR. I've gone through it and it looks good to merge except for some small issues.

hash_template = '{method}%s{stub}{param_str}' % os.sep

This would add an extra slash to hash_template and as a result, create an extra directory as a side-effect. hash_template would be something like GET/index.html instead of GET:index.html and make_dirs function would create an extra directory called GET.

While this works without any issues, I'd rather have this documented somewhere than have this as a side-effect. Maybe we could use _ or - as a separator between {method} and {stub}

@cyriac what do you think ?

sebinthomas avatar Jan 06 '19 18:01 sebinthomas

Hi @laerreal. Thanks for the contribution.

As @sebinthomas suggested, this would cause one additional directory to be created that is not necessarily adding value. Could we use a different separator there?

Also instead of using os.sep.join can we use os.path.join to keep it consistent?

cyriac avatar Jan 07 '19 06:01 cyriac

Hi. Using slash as seperator also preserves original file names. This can be convenient under some circumstances. I also noticed yet another problem. It's with using of ? for param string. That symbol is also forbidden under Windows. Also, suffxing anything after file extension will confuse Windows about file type. Of course, a NIX OS is robust against those problems.

So, I suggest next:

  1. use (space) as a separator for both method and its parameters.
  2. place parameters between method and stub:
'{method} {param_str} {stub}'

Also, is it possible in HTTP request, a raw space is present in either file name or parametrs? If not, this approch will also revent possible confusion with requests of files with names like "GET-myfile.html".

What do you think?

P.S. I will replace os.sep.join with os.path.join.

laerreal avatar Jan 07 '19 11:01 laerreal

How about we convert get_hashed_filepath to return an md5 or sha1 hash of the resultant string? This would avoid all of the problems.

So the code would roughly translate to something like this

def get_hashed_filepath(stub, method, parsed_url, params):
    hash_template = '{method}%s{stub}{param_str}' % os.sep  # Formatting for this can be anything

    ... other code ....

    filepath = hash_template.format(method=method, stub=stub, param_str=param_str)
    return hashlib.md5(filepath).hexdigest()

cyriac avatar Jan 08 '19 05:01 cyriac

This approach results in hiding of file names.

default

I think, at least, the hash have to be used as a directory name. But it's appearance issue only.

laerreal avatar Jan 09 '19 10:01 laerreal

@laerreal I suggest you keep

hash_template = '{method}_{stub}{param_str}'
hash_str  = hash_template.format(method=method, stub=stub, param_str=param_str)

and do

return md5(hash_str).hexdigest()

This makes it easy if you want to revert or see the filename/stub if you're debugging. Notice the _ after {method} instead of os.sep

Looks good to merge otherwise.

sebinthomas avatar Jan 09 '19 16:01 sebinthomas