Skip to content
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

sftp server, file browser #145

Merged
merged 1 commit into from Dec 26, 2014
Merged

sftp server, file browser #145

merged 1 commit into from Dec 26, 2014

Conversation

@haliphax
Copy link
Collaborator

haliphax commented Dec 14, 2014

configuration

to configure sftp server:

  • add [sftp] section to default.ini
  • add root option to [sftp] section, pointing at root folder for files
  • (optionally) add umask option to [sftp] section for default umask on uploaded files

to enable uploads:

  • create __uploads__ directory in sftp root

to enable file browser:

  • add menu option for gosub('fbrowse')

things to add

  • FILE_ID.DIZ caching, but that seems like premature optimization at this point
  • ability to enter manual descriptions for files (will depend on same storage mechanism that holds FILE_ID.DIZ cache)
  • virtual FILES.BBS generation so that sftp clients can view descriptions of a folder's files without being logged in via terminal server
  • integration with x/84's IPC logging mechanism
  • better detection of file download completion (currently, file is removed from user's tagged list as soon as it is opened via sftp, rather than when it is finished)
  • tracking uploads/downloads (i.e., who uploaded what, how many times has something been downloaded, who downloaded what, etc.)

notes

  • this adds filesize to common.py for a human-readable file size function as per #146
# @TODO add support for DMS archives


""" archive extraction functions """

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

this would become a docstring for the module, following "File browsing/tagging for..."

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

that is, nix it, or change it to ## comments

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

I understand that it's the docstring. Is it wrong? Or are you referring to the "archive extraction functions" portion? Hard to tell based on what you've highlighted.

This comment has been minimized.

Copy link
@jquast

jquast Dec 16, 2014

Owner

the doc builds aren't very easy right now, I'll work on them again in the coming weeks -- But the API documents would have an "overview" or top-level description, and currently it would read:

File browsing/tagging for x/84 bbs https://github.com/jquast/x84

archive extraction functions

config

script logic

followed by all of the methods/functions here. As these are on the top-level (no indent), they are not attatched to anything in particular, just the top-level module

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 18, 2014

Author Collaborator

Gotcha. That was the clarification I needed. I'll move them into # comments, instead.

return unzip_zip(filename, method=zipfile.ZIP_BZIP2)


""" config """

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

as this

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

Referring to "config" ?

return u'No description'
except zipfile.BadZipfile:
return u'Bad zip file, cannot parse'
except zipfile.LargeZipFile:

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

why not use zipfile.LargeZipFile, and avoid such exception/error

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

Didn't see that scenario dealt with in the docs I was reading, but I'll hammer it out.

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 18, 2014

Author Collaborator

LargeZipFile is not a usable class; it is an Exception subclass. At any rate, I found there's a flag, allowZip64, that you can pass to the zipfile.ZipFile constructor to allow for Zip64, which should bypass the problem.


""" archive extraction functions """

def unzip_zip(filename, method=zipfile.ZIP_STORED):

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

these should all be called something like extract_fileid_diz or some such

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

Agreed

tagged_dirname = u'__tagged__%s' % os.path.sep


""" script logic """

This comment has been minimized.

Copy link
@jquast

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

You lost me.

it has to be done this way because these are scalar values, not pointers
nor classes, etc., and they change when screen dimensions change.
maybe a dict would be better? not sure.

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

maybe something like an enum,

FileBrowseVariables = collections.namedtuple('fb_defs',
('diz_location', 'last_diz_len', 'max_diz_width', 'max_diz_height', 'tagged_files')
)(0, 0, 0, 0, tagged_files=set())

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 18, 2014

Author Collaborator

namedtuple is immutable; didn't work

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 18, 2014

Author Collaborator

dict works.

@@ -0,0 +1,360 @@
""" File browsing/tagging for x/84 bbs https://github.com/jquast/x84 """

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

prefer 'fbrowser.py'

fullname = os.path.join(directory, filename)
stat = os.stat(fullname)
filesize = None
if stat.st_size > 1024 * 400:

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

could have sworn we already wrote this, but i'm confusing with 'timeago' i guess.

anyway, this "human readable" should be separate utility function, and would do better to use pow(1024, 2), or better yet, here is an awk script that "upsamples" or "downsamples" any human-readable format to the designated format. Notice it uses 1024 all the way -- not sure what is up with these 1024000, 1024*400, etc. variables.

~
#!/bin/ksh
# dingo@efnet
nawk -v scale=$1 -v precision=$2 '
BEGIN {
  # f_bytes and b_bytes, forwards and backwards for up and down-scaling
  f_bytes="B K M G T P E Z"
  b_bytes="Z E P T G M K B"

  usage=0 # flipped on error condition, exit with usage
  if ((scale == "-h") || (scale == "--help"))
    usage=1
  split(f_bytes, b, " ");
  split(scale, chk_scale, "");
  badscale=1;
  if (!length(chk_scale))
    { scale="B"; badscale=0; } # default output is bytes
  else
    for(i=1;i<length(b);i++)
    {
      if (toupper(chk_scale[1]) == b[i])
        { badscale=0; scale=b[i]; break; printf ("scale now %s\n", scale);}
    }
  if (badscale) {
    printf("Bad scale: %s, must one of: %s\n", scale, f_bytes);
    usage=1
  }

  # still, no size checking etc., but we make sure its an integer!
  split(precision, chk_precision, "");
  if (!length(chk_precision))
    precision=0;
  else
    for(i=1;i<length(chk_precision);i++) {
      if (match(chk_precision[i],"[0-9]") == 0)
        { printf("Bad precision: %s, must be integer\n", precision); usage=1; break }
    }

  if (usage)
  {
    printf("\n usage: %s [scale] [precision]\n\n", argv[0]);
    printf("program recieves numeric input from stdin as bytesize\n");
    printf("of scale in each input line. If scale is specified, input\n");
    printf("is up or down-scaled, otherwise input is always downscaled to bytes.\n");
    printf("scales are any of: %s\n\n", f_bytes);
    printf("upper or lowercase.\n");
    printf("if input is upscaled, meaning precision is lost, then the\n");
    printf("second option argument is used to determine the float\n");
    printf("decimal precision used. default is 0, meaning no decimals.\n\n");
    exit(1);
  }
} {
  upscaled=0;
  split(f_bytes, b, " ")
  p=match($0,"[ZzEePpTtGgMmKkBb]");
  if (!p)
  {
    t="B"; # unspecified sizes presumed to be 'B'
    d=$1
  } else {
    t=substr($0, p, 1);
    d=substr($0, 0, p-1);
  }

  for(i=1;i<length(b);i++)
    if (toupper(t)==b[i])
    {
      upscaled=1;
      d/=1024;
      t=b[i+1];
      if (toupper(t) == toupper(scale))
        break;
    }

  if (toupper(t) != toupper(scale)) {
    split(b_bytes, b, " ");
    for(i=1;i<length(b);i++)
      if (toupper(t)==b[i])
      {
        d*=1024;
        t=b[i+1];
        if (toupper(t) == toupper(scale))
          break;
      }
  }

  if (upscaled)
    printf ("%0.*f%s\n", precision, d, scale);
  else
    printf ("%i%s\n", d, scale);
}'

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

It's just that it would say "0.5M" instead of "500K" if it's above 400K. That's all the 1024 * 400 is about. The multiplication isn't necessary, but is there for readability.

This comment has been minimized.

Copy link
@jquast

jquast Dec 16, 2014

Owner

leave it for now and i will suggest general utility function later

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 18, 2014

Author Collaborator

Well, I can turn it into a function and throw it in common.py without much effort.

x84/sftp.py Outdated
def __init__(self, *args, **kwargs):
self.log = logging.getLogger('x84.engine')
self.user = kwargs['user']
del kwargs['user']

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

little creepy, should be: self.user = kwargs.pop('user')

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

Noted

x84/sftp.py Outdated
try:
SFTPServer.set_file_attr(self.filename, attr)
return SFTP_OK
except OSError as e:

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

pylint: variables should be 3-letters (such as err)

x84/sftp.py Outdated
def __init__(self, *args, **kwargs):
from x84.bbs import DBProxy
self.ssh_session = kwargs['session']
del kwargs['session']

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

self.ssh_session = kwargs.pop('session')

x84/sftp.py Outdated

# Copyright (C) 2003-2009 Robey Pointer <robeypointer@gmail.com>
#
# This file is part of paramiko.

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

Is it, now? I don't think we need this legalise, the derived class of SFTPServerInterface is our code, unless its copy and pasted, in which case, why can't we call super for those?

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

I'll have to check the Paramiko license to see what we do here. I just took his StubSFTPServer and changed some stuff around.

This comment has been minimized.

Copy link
@jquast

jquast Dec 16, 2014

Owner

ah that explains this strange stuff, didn't look like your style.

x84/sftp.py Outdated
self.log.debug('list_folder({0!r})'.format(path))
rpath = self._realpath(path)
if u'sysop' not in self.user.groups and \
u'/%s' % uploads_dirname == path:

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

pretty dangerous grounds here. Suggest not to use % at all, preferring u'/{0}'.format(uploads_dirname)

but if you do use %, please write like:

u'/%s' % (uploads_dirname,)

especially here, we're looking at the possibility of u'/True' if our order of precedence is mistaken.

x84/sftp.py Outdated
(uploads_dirname in path and os.path.exists(path)):
return SFTP_PERMISSION_DENIED
try:
binary_flag = getattr(os, 'O_BINARY', 0)

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

does os.O_BINARY sometimes not exist??

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

No idea. StubSFTPServer original code here.

This comment has been minimized.

Copy link
@jquast

jquast Dec 16, 2014

Owner

retracted,

probably some windows or python2.5 or some such compatibility

x84/sftp.py Outdated
else:
# os.open() defaults to 0777 which is
# an odd default mode for files
fd = os.open(path, flags, 0o666)

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

ok... so we are then using 0o444 with this umask, correct? can we say so?

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

I don't follow. StubSFTPServer has it set for 0o666. Are you saying you want 0o444?

This comment has been minimized.

Copy link
@jquast

jquast Dec 16, 2014

Owner

well this is setting file permissions to 0o444, yes. Do we want that? I don't know

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 18, 2014

Author Collaborator

I must be missing something. It looks like it attempts to set the mode to the same as the uploaded file (via getattr) and if it can't pull the mode at all, it defaults to 0o666. I'll do some testing to see if that's true.

if flags & os.O_APPEND:
fstr = 'ab'
else:
fstr = 'wb'

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

is it ok for non-sysops to write over existing files?

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

The idea was that nobody can see what's listed in uploads, so if they for some reason wanted to upload a newer version of something they just uploaded, they could. I'm not married to the idea.

This comment has been minimized.

Copy link
@jquast

jquast Dec 16, 2014

Owner

to move it from uploads, this would be done manually by the sysop?

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 18, 2014

Author Collaborator

Yes.

Edit: Clarification - sysops can see the contents of the uploads folder. Ordinary users cannot.

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 19, 2014

Author Collaborator

there is now a umask option in [sftp] section of default.ini for setting the default umask, and if it is not present, it will default to a more-sane 0o664.

x84/sftp.py Outdated
return SFTP_OK

def mkdir(self, path, attr):
if not u'sysop' in self.user.groups or tagged_dirname in path:

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

heavily suggest using self.user.is_sysop in all such cases, this is a common pattern

self.log.debug('lstat({0!r}, {1!r}, {2!r})'
.format(path, flags, attr))
path = self._realpath(path)
if flags & os.O_CREAT and (uploads_dirname not in path and \

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

suggest is_uploaddir(path) function return Bool

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

Agreed

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 19, 2014

Author Collaborator

Done (except in this case, where it only needs to tell if this file is within the uploads dir [or a subdir])



def mark_tagged(directory, files):
""" add marker to tagged files """

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

I think tagged is the wrong phrase here, this is not like messagebase tags, this is a download or save list or some such. I would like to see a real file tag, thats global, so that instead of file areas, you have "amiga" and "pc-dos", "util", etc. tags applied, and browse virtual folders of such tags by sftp, or such category tags by interface.

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

Depends on where you're wanting to go with the file server. I just saw it as a folder-based SFTP server. "Flagged", maybe?

This comment has been minimized.

Copy link
@jquast

jquast Dec 16, 2014

Owner

flagged is good.

I do plan to go with "tagging" for files -- loosely, I'd see a "pirate bbs" to allow:

  • haliphax makes new group tag, "warezg0dz"
  • haliphax marks group tag "member-private"
  • haliphax invites dingo
  • dingo uploads "Heroes of Might and Magic III.rar"
  • dingo tags this file "warezg0dz"
  • only dingo and haliphax may see the file

Allows users to create their own public and private "file conferences" without sysop access. That's the idea i have in store, anyway.

x84/ssh.py Outdated
@@ -260,9 +276,10 @@ def _timeleft(self, st_time):
time.time() - st_time < self.TIME_WAIT_STAGE)


class SshSessionServer(paramiko.ServerInterface):
class SshSessionServer(paramiko.ServerInterface, ):

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

woops trailing comma?

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 18, 2014

Author Collaborator

Wasn't me. :)

'zip': unzip_zip,
'bz2': unzip_bzip2,
'bzip2': unzip_bzip2,
}

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

why is .tar, .tgz, .tar.gz, .tar.bz not possible?

is .bzip2 legitimate extension? i have never seen it

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

I only added *.zip because it is the most likely non-ARJ, non-RAR file that will have a FILE_ID.DIZ. I haven't known *.tgz files to have FILE_ID.DIZ. bzip2 archives were added because zipfile supports them out of the box, and that's what's handling *.zip.

""" config """

# map extensions to helper functions for pulling FILE_ID.DIZ
supported_archives = {

This comment has been minimized.

Copy link
@jquast

jquast Dec 15, 2014

Owner

again, supported_archives is more like file_id_extract_functionmap

This comment has been minimized.

Copy link
@haliphax

haliphax Dec 16, 2014

Author Collaborator

Depends on your perspective. It's used later in the code like "if ext in supported_archives", which makes logical sense. But either way, I'm not ideologically opposed to renaming it to make sense in a different context.

jquast added a commit that referenced this pull request Dec 26, 2014
@jquast jquast merged commit 82d1e96 into jquast:master Dec 26, 2014
@jquast

This comment has been minimized.

Copy link
Owner

jquast commented Dec 26, 2014

big brother thanks you, comrade

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.