diff options
| -rw-r--r-- | Makefile | 5 | ||||
| -rw-r--r-- | TODO.md | 88 | ||||
| -rw-r--r-- | tmp.go | 130 | ||||
| -rwxr-xr-x | tmp.py | 115 | ||||
| -rwxr-xr-x | tmp.sh | 165 |
5 files changed, 83 insertions, 420 deletions
diff --git a/Makefile b/Makefile deleted file mode 100644 index 4f8ad1c..0000000 --- a/Makefile +++ /dev/null @@ -1,5 +0,0 @@ -tmp: tmp.go - go build $< - -clean: - rm -f tmp *~ diff --git a/TODO.md b/TODO.md deleted file mode 100644 index 49244c4..0000000 --- a/TODO.md +++ /dev/null @@ -1,88 +0,0 @@ -### Possible Bugs -1. **Missing URL Variants Handling**: The code retrieves the - configuration for the command name but does not handle cases where the - command name does not exist in `URL_VARIANTS`, which could lead to - a `nil` dereference. -2. **Error Handling**: The program exits with `os.Exit(1)` if no - arguments are provided, but it does not log a message indicating why it - exited, which could confuse users. -3. **Compression Command**: The code assumes that the `gzip` command is - available in the system's PATH. If it is not, the program will fail - without a clear error message. - -### Possible Improvements -1. **Error Messages**: Add more descriptive error messages when exiting - due to missing command variants or file arguments. -2. **Configuration Validation**: Implement checks to ensure that the - `URL_VARIANTS` map contains the necessary keys before accessing them. -3. **File Compression Feedback**: Provide feedback on successful - compression, such as logging the new file name created. -4. **Command-Line Flags**: Consider using a command-line flag library to - improve argument parsing and provide help messages. - -### Potential Security Concerns -1. **Command Injection**: The use of `exec.Command` with user-provided - input could lead to command injection vulnerabilities if not properly - sanitized. Although the current implementation does not directly take - user input for command execution, it is a consideration for future - modifications. -2. **File Permissions**: The program does not check for file permissions - before attempting to read or compress files, which could lead to - permission errors if the user does not have the necessary rights. -3. **Resource Exhaustion**: If a large number of files are processed, - the program could exhaust system resources (e.g., memory or file - descriptors) if not properly managed, especially if the compression - command spawns many subprocesses. - -### Error Handling Analysis -1. **Use of `log.Fatal`**: The code employs `log.Fatal(err)` to handle - errors, which logs the error message and exits the program. While this - is effective for critical failures, it does not allow for graceful - recovery or error reporting to the user. -3. **Exit on Missing Arguments**: The program exits with `os.Exit(1)` if - no arguments are provided, which is a straightforward approach but could - be improved by providing a user-friendly message indicating the expected - usage. - -### Concurrency and Threading -1. **Single-threaded Execution**: The code processes files sequentially - without any concurrency or parallelism. For large numbers of files, this - could lead to performance bottlenecks. -2. **Potential for Blocking**: The use of `exec.Command` to run external - commands (like `gzip`) can block the main thread until the command - completes, further impacting performance. - -### Refactoring Suggestions -1. **Error Handling Improvement**: Instead of using `log.Fatal`, - consider returning errors to the caller or logging them with more - context. This would allow for better error management and user feedback. -2. **Command Execution Handling**: Wrap the command execution in - a function that can handle retries or provide more detailed error - messages. -3. **Concurrency Implementation**: Introduce goroutines to process files - concurrently, which would significantly improve performance when dealing - with multiple files. -4. **Configuration Management**: Consider externalizing the - `URL_VARIANTS` configuration to a separate file or environment variables - for easier management and updates. - -### Comparisons with Best Practices -1. **Logging Practices**: While logging is used, it could be enhanced by - including timestamps and log levels (info, warning, error) for better - traceability. -2. **Function Modularity**: The `main` function is doing multiple tasks - (argument parsing, file processing, logging), which could be broken down - into smaller, more focused functions to improve readability and - maintainability. - -### Collaboration and Readability -3. **Error Messages**: Providing more informative error messages would - enhance collaboration, as other developers would have a clearer - understanding of issues that arise. -4. **Documentation**: Including a README or documentation that explains - how to use the program, its dependencies, and its configuration would - greatly benefit other developers and users. - -Overall, while the code is functional, there are several areas for -improvement in error handling, concurrency, and readability that could -enhance its robustness and maintainability. @@ -1,130 +0,0 @@ -package main - -import ( - "fmt" - "log" - "log/slog" - "net/url" - "os" - "os/exec" - "path/filepath" - "strings" -) - -var COMPRESSED_EXTENSIONS = []string{".jpg", ".webm", ".ogg", ".gz", ".bz2", ".zip", - ".xz", ".odp", ".odt", ".ods", ".mp4", ".mp3", - ".png", ".pdf", ".apk", ".rpm", ".epub", ".mkv", - ".avi", ".jar", ".opus", ".ogv", ".flac", ".msi", - ".m4a"} - -// THE VARIABLE URL_VARIANTS IS MEANT TO BE EDITED. Change various -// attributes to correspond to your situation. -// If new section is added (as here I have also 'tmp' for fedorapeople.org) -// and the script has a symlink named as the section (so I have tmp as -// a symlink to this script) it saves the file to the location defined in -// such section when called under the other name. -// So, `tmp filename` saves file filename to fedorapeople for me. -var URL_VARIANTS = map[string]map[string]string{ - "wotan": { - "base_url": "wotan:Export/", - "target_url": "https://w3.suse.de/~mcepl/", - }, - "tmp": { - "base_url": "fedorapeople.org:public_html/tmp/", - "target_url": "https://mcepl.fedorapeople.org/tmp/", - // "shorten_api": "http://is.gd/create.php?format=simple&url=" - "shorten_api": "https://da.gd/s?url=", - }, -} - -func contains(slice []string, item string) bool { - for _, a := range slice { - if a == item { - return true - } - } - return false -} - -func main() { - if len(os.Args) < 2 { - os.Exit(1) - } - - slog.SetLogLoggerLevel(slog.LevelInfo) - - cmdname := strings.ToLower(filepath.Base(os.Args[0])) - config := URL_VARIANTS[cmdname] - - if _, err := os.Stat(os.Args[1]); os.IsNotExist(err) { - os.Exit(1) - } - - for _, processedFile := range os.Args[1:] { - st, err := os.Stat(processedFile) - if err != nil { - log.Fatal(err) - } - fileExt := filepath.Ext(processedFile) - slog.Debug(fmt.Sprintf("size of %s is %d.", processedFile, st.Size())) - slog.Debug(fmt.Sprintf("extension is %s.", fileExt)) - - var fname string - var COMPRESSED bool - - if st.Size() > 1000000 && !contains(COMPRESSED_EXTENSIONS, strings.ToLower(fileExt)) { - slog.Debug(fmt.Sprintf("file %s should be compressed (%d b)", processedFile, st.Size())) - err := exec.Command("gzip", "--keep", "--best", processedFile).Run() - if err != nil { - log.Fatal("Compressing the file failed!") - } - fname = processedFile + ".gz" - COMPRESSED = true - } else { - slog.Debug(fmt.Sprintf("file %s should not be compressed (%d b)", processedFile, st.Size())) - fname = processedFile - COMPRESSED = false - } - - err = os.Chmod(fname, 0o644) - if err != nil { - log.Fatal(err) - } - // To make scp happy - modifiedFname := strings.Replace(fname, ":", "_", 1) - err = os.Rename(fname, modifiedFname) - if err != nil { - log.Fatal(err) - } - slog.Debug(fmt.Sprintf("Unshorten fname = %s", modifiedFname)) - - _, err = exec.Command("scp", modifiedFname, config["base_url"]).Output() - if err != nil { - log.Fatal(err) - } - - targetURL := config["target_url"] + filepath.Base(modifiedFname) - // Make target_URL into a safe URL - safeTargetURL := url.QueryEscape(targetURL) - safeTargetURL = strings.Replace(safeTargetURL, "%3A", ":", 1) - fmt.Println(targetURL) - - // curl -s 'http://is.gd/create.php?format=simple&url=www.example.com' - // http://is.gd/MOgh5q - if shortenAPI, ok := config["shorten_api"]; ok { - shortenedURLBytes, err := exec.Command("curl", "-s", shortenAPI+safeTargetURL).Output() - if err != nil { - log.Fatal(err) - } - shortenedURL := strings.TrimSpace(string(shortenedURLBytes)) - fmt.Println(shortenedURL) - } - - // The script should have no side-effects - if COMPRESSED { - os.Remove(modifiedFname) - } else { - os.Rename(modifiedFname, fname) - } - } -} @@ -1,115 +0,0 @@ -#!/bin/bash -# -# "THE BEER-WARE LICENSE" (Revision 42): -# Matěj Cepl, <mcepl@cepl.eu> wrote this file. As long as you retain -# this notice you can do whatever you want with this stuff. If we meet -# some day, and you think this stuff is worth it, you can buy me a beer -# in return. Matěj Cepl -# -# Instructions for setting up appropriate folders on -# file.rdu.redhat.com see https://mojo.redhat.com/docs/DOC-14590 -# See also variable URL_VARIANTS below for necessary configuration. -set -eu - -DEBUG=1 - -debug() { - if [ $DEBUG -eq 1 ] ; then - printf "%s\n" "$@" - fi -} - -error() { - printf "%s\n" "$@" - exit 1 -} - -COMPRESSED_EXTENSIONS=('.jpg' '.webm' '.ogg' '.gz' '.bz2' '.zip' \ - '.xz' '.odp' '.odt' '.ods' '.mp4' '.mp3' \ - '.png' '.pdf' '.apk' '.rpm' '.epub' '.mkv' \ - '.avi' '.jar' '.opus' '.ogv' '.flac' '.msi' \ - '.m4a') - -# THE VARIABLE URL_VARIANTS IS MEANT TO BE EDITED. Change various -# attributes to correspond to your situation. -# If new section is added (as here I have also 'tmp' for fedorapeople.org) -# and the script has a symlink named as the section (so I have tmp as -# a symlink to this script) it saves the file to the location defined in -# such section when called under the other name. -# So, `tmp filename` saves file filename to fedorapeople for me. - -export URL_VARIANTS="{ - 'barstool': { - 'base_url': 'mcepl@shell.eng.rdu.redhat.com:public_html/', - 'target_url': 'http://file.rdu.redhat.com/~mcepl/', - 'shorten_api': 'https://url.corp.redhat.com/new?' - }, - 'wotan': { - 'base_url': 'wotan:Export/', - 'target_url': 'https://w3.suse.de/~mcepl/' - }, - 'tmp': { - 'base_url': 'fedorapeople.org:public_html/tmp/', - 'target_url': 'https://mcepl.fedorapeople.org/tmp/', - 'shorten_api': 'https://da.gd/s?url=' - } -}" - -# Requires GNU sed -CMDNAME="$(basename "$0"|sed 's/./\L&/g')" -CONFIG="$(echo "$URL_VARIANTS" |tr "'" '"'|jq -r ".$CMDNAME")" -BASE_URL="$(echo "$CONFIG" | jq -r ".base_url")" -TARGET_URL="$(echo "$CONFIG" | jq -r ".target_url")" - -[ $# -lt 1 ] && exit 1 - -for processed_file in "${@:1}" ; do - st=$(stat -c "%s" "$processed_file") - base_name="$(basename -- "$processed_file")" - file_ext="${base_name##*.}" - debug "size of $processed_file is ${st} bytes" - debug "extension is ${file_ext}" - - # shellcheck disable=SC2076 - if [ "$st" -gt 1000000 ] && [[ ! " ${COMPRESSED_EXTENSIONS[*]} " =~ " ${file_ext} " ]] ; then - debug "file ${processed_file} should be compressed (${st} b)" - gzip --keep --best "${processed_file}" || error 'Compressing the file failed!' - fname="${processed_file}.gz" - COMPRESSED=1 - else - debug "file ${processed_file} should not be compressed (${st} b)" - fname="${processed_file}" - COMPRESSED=0 - fi - - chmod 644 "$fname" - - # To make scp happy - modified_fname="$(echo "${fname}"|tr ':' '_')" - mv -n "${fname}" "${modified_fname}" - - debug "Unshorten fname=${modified_fname}" - - scp "${modified_fname}" "${BASE_URL}" - - target_URL="${TARGET_URL}$(basename "${modified_fname}")" - - # Make target_URL into a safe URL - safe_target_URL="${target_URL// /%20}" - echo "${safe_target_URL}" - - # curl -s 'http://is.gd/create.php?format=simple&url=www.example.com' - # http://is.gd/MOgh5q - SHORTEN_API="$(echo "$CONFIG" | jq -r ".shorten_api")" - if [ "x${SHORTEN_API}" != "xnull" ] ; then - shortened_URL="$(curl -s "${SHORTEN_API}""${safe_target_URL}")" - echo "${shortened_URL}" - fi - - # The script should have no side-effects - if [ "${COMPRESSED}" -eq 1 ] ; then - rm "${modified_fname}" - else - mv "${modified_fname}" "${fname}" - fi -done @@ -1,4 +1,4 @@ -#!/usr/bin/python3 +#!/bin/bash # # "THE BEER-WARE LICENSE" (Revision 42): # Matěj Cepl, <mcepl@cepl.eu> wrote this file. As long as you retain @@ -9,21 +9,26 @@ # Instructions for setting up appropriate folders on # file.rdu.redhat.com see https://mojo.redhat.com/docs/DOC-14590 # See also variable URL_VARIANTS below for necessary configuration. +set -eu -import logging -import os -import os.path -import subprocess -import sys -import urllib.parse -logging.basicConfig(format='%(levelname)s:%(funcName)s:%(message)s', - level=logging.INFO) - -COMPRESSED_EXTENSIONS = ['.jpg', '.webm', '.ogg', '.gz', '.bz2', '.zip', - '.xz', '.odp', '.odt', '.ods', '.mp4', '.mp3', - '.png', '.pdf', '.apk', '.rpm', '.epub', '.mkv', - '.avi', '.jar', '.opus', '.ogv', '.flac', '.msi', - '.m4a'] +DEBUG=1 + +debug() { + if [ $DEBUG -eq 1 ] ; then + printf "%s\n" "$@" + fi +} + +error() { + printf "%s\n" "$@" + exit 1 +} + +COMPRESSED_EXTENSIONS=('.jpg' '.webm' '.ogg' '.gz' '.bz2' '.zip' \ + '.xz' '.odp' '.odt' '.ods' '.mp4' '.mp3' \ + '.png' '.pdf' '.apk' '.rpm' '.epub' '.mkv' \ + '.avi' '.jar' '.opus' '.ogv' '.flac' '.msi' \ + '.m4a') # THE VARIABLE URL_VARIANTS IS MEANT TO BE EDITED. Change various # attributes to correspond to your situation. @@ -32,83 +37,79 @@ COMPRESSED_EXTENSIONS = ['.jpg', '.webm', '.ogg', '.gz', '.bz2', '.zip', # a symlink to this script) it saves the file to the location defined in # such section when called under the other name. # So, `tmp filename` saves file filename to fedorapeople for me. -URL_VARIANTS = { - 'barstool': { - 'base_url': 'mcepl@shell.eng.rdu.redhat.com:public_html/', - 'target_url': 'http://file.rdu.redhat.com/~mcepl/', - 'shorten_api': 'https://url.corp.redhat.com/new?' - }, - 'wotan': { - 'base_url': 'wotan:Export/', - 'target_url': 'https://w3.suse.de/~mcepl/' - }, - 'tmp': { - 'base_url': 'fedorapeople.org:public_html/tmp/', - 'target_url': 'https://mcepl.fedorapeople.org/tmp/', - # 'shorten_api': 'http://is.gd/create.php?format=simple&url=' - 'shorten_api': 'https://da.gd/s?url=' - } -} -cmdname = os.path.basename(sys.argv[0]).lower() -config = URL_VARIANTS[cmdname] - -if not os.path.exists(sys.argv[1]): - sys.exit(1) - -for processed_file in sys.argv[1:]: - st = os.stat(processed_file).st_size - file_ext = os.path.splitext(processed_file)[1] - logging.debug('size of {} is {:,}.'.format(processed_file, st)) - logging.debug('extension is {}'.format(file_ext)) - if st > 1000000 and file_ext.lower() not in COMPRESSED_EXTENSIONS: - logging.debug( - "file {} should be compressed ({:,} b)".format( - processed_file, st)) - try: - ret = subprocess.check_call( - ['gzip', '--keep', '--best', processed_file]) - except subprocess.CalledProcessError: - logging.error('Compressing the file failed!') - raise - fname = processed_file + '.gz' - COMPRESSED = True - else: - logging.debug( - 'file {} should not be compressed ({:,} b)'.format( - processed_file, st)) - fname = processed_file - COMPRESSED = False - - os.chmod(fname, 0o0644) +export URL_VARIANTS="{ + 'barstool': { + 'base_url': 'mcepl@shell.eng.rdu.redhat.com:public_html/', + 'target_url': 'http://file.rdu.redhat.com/~mcepl/', + 'shorten_api': 'https://url.corp.redhat.com/new?' + }, + 'wotan': { + 'base_url': 'wotan:Export/', + 'target_url': 'https://w3.suse.de/~mcepl/' + }, + 'tmp': { + 'base_url': 'fedorapeople.org:public_html/tmp/', + 'target_url': 'https://mcepl.fedorapeople.org/tmp/', + 'shorten_api': 'https://da.gd/s?url=' + } +}" + +# Requires GNU sed +CMDNAME="$(basename "$0"|sed 's/./\L&/g')" +CONFIG="$(echo "$URL_VARIANTS" |tr "'" '"'|jq -r ".$CMDNAME")" +BASE_URL="$(echo "$CONFIG" | jq -r ".base_url")" +TARGET_URL="$(echo "$CONFIG" | jq -r ".target_url")" + +[ $# -lt 1 ] && exit 1 + +for processed_file in "${@:1}" ; do + st=$(stat -c "%s" "$processed_file") + base_name="$(basename -- "$processed_file")" + file_ext="${base_name##*.}" + debug "size of $processed_file is ${st} bytes" + debug "extension is ${file_ext}" + + # shellcheck disable=SC2076 + if [ "$st" -gt 1000000 ] && [[ ! " ${COMPRESSED_EXTENSIONS[*]} " =~ " ${file_ext} " ]] ; then + debug "file ${processed_file} should be compressed (${st} b)" + gzip --keep --best "${processed_file}" || error 'Compressing the file failed!' + fname="${processed_file}.gz" + COMPRESSED=1 + else + debug "file ${processed_file} should not be compressed (${st} b)" + fname="${processed_file}" + COMPRESSED=0 + fi + + chmod 644 "$fname" # To make scp happy - modified_fname = fname.replace(':', '_') - os.rename(fname, modified_fname) + modified_fname="$(echo "${fname}"|tr ':' '_')" + mv -n "${fname}" "${modified_fname}" - logging.debug('Unshorten fname = {}'.format(modified_fname)) + debug "Unshorten fname=${modified_fname}" - subprocess.check_call(['scp', '{}'.format(modified_fname), - config['base_url']]) + scp "${modified_fname}" "${BASE_URL}" - target_URL = config['target_url'] + os.path.basename(modified_fname) + target_URL="${TARGET_URL}$(basename "${modified_fname}")" # Make target_URL into a safe URL - safe_target_URL = urllib.parse.quote(target_URL).replace('%3A', ':', 1) - - print(safe_target_URL) + safe_target_URL="${target_URL// /%20}" + echo "${safe_target_URL}" # curl -s 'http://is.gd/create.php?format=simple&url=www.example.com' # http://is.gd/MOgh5q - - if 'shorten_api' in config: - shortened_URL = subprocess.check_output( - ['curl', '-s', config['shorten_api'] + - safe_target_URL]).decode().strip() - print(shortened_URL) + SHORTEN_API="$(echo "$CONFIG" | jq -r ".shorten_api")" + if [ "x${SHORTEN_API}" != "xnull" ] ; then + shortened_URL="$(curl -s "${SHORTEN_API}""${safe_target_URL}")" + echo "${shortened_URL}" + fi # The script should have no side-effects - if COMPRESSED: - os.unlink(modified_fname) - else: - os.rename(modified_fname, fname) + if [ "${COMPRESSED}" -eq 1 ] ; then + rm "${modified_fname}" + else + mv "${modified_fname}" "${fname}" + fi +done |