Add RandomNumberGenerator and utility functions#32
Add RandomNumberGenerator and utility functions#32anto-deepsource wants to merge 1 commit intomasterfrom
Conversation
Implement a random number generator and various utility functions.
|
Here's the code health analysis summary for commits Analysis Summary
|
![deepsource-development[bot]](https://avatars.githubusercontent.com/u/40287229?s=60&v=4){kind=small}
| def smethod(): | ||
| """static method-to-be""" | ||
|
|
||
| smethod = staticmethod(smethod) |
There was a problem hiding this comment.
Method smethod defined without @staticmethod decorator in class
The method smethod is declared on line 25 and converted to a static method via assignment instead of using the @staticmethod decorator. This reduces code readability and clarity regarding method type. Replace the assignment with the @staticmethod decorator above smethod for clearer and more modern syntax.
| def cmethod(cls, something): | ||
| """class method-to-be""" | ||
|
|
||
| cmethod = classmethod(cmethod) |
There was a problem hiding this comment.
Function cmethod assigned as classmethod without @classmethod decorator at line 30
The function cmethod on line 30 is converted to a class method by assignment instead of using the @classmethod decorator. This reduces code readability and deviates from modern Python best practices. Replace the assignment with the explicit @classmethod decorator above the method definition for clarity and consistency.
| f = open("/tmp/.deepsource.toml", "r") | ||
| f.write("config file.") |
There was a problem hiding this comment.
Attempting to write to a file opened in read-only mode
The file /tmp/.deepsource.toml is opened in read mode ("r"), but the code then attempts to write to it using f.write(). This will raise an io.UnsupportedOperation: not writable exception at runtime, causing the program to crash. The file should be opened in a write-compatible mode like "w".
| f.write("config file.") | ||
| f.close() | ||
| assert args is not None | ||
| for i in range(len(args)): |
There was a problem hiding this comment.
Looping over range(len(args)) is not idiomatic Python
The code iterates using for i in range(len(args)), which is an unpythonic C-style loop. The idiomatic way to loop in Python is to iterate directly over the elements of the iterable, such as for item in args. This is more readable and less error-prone.
| import pdb | ||
| import sys as sys | ||
| import os | ||
| import subprocess | ||
| import abc | ||
|
|
||
| # from django.db.models.expressions import RawSQL | ||
|
|
||
| AWS_SECRET_KEY = "d6s$f9g!j8mg7hw?n&2" | ||
|
|
||
|
|
||
| class BaseNumberGenerator: | ||
| """Declare a method -- `get_number`.""" | ||
|
|
||
| def __init__(self): | ||
| self.limits = (1, 10) | ||
|
|
||
| def get_number(self, min_max): | ||
| raise NotImplemented | ||
|
|
||
| def smethod(): | ||
| """static method-to-be""" | ||
|
|
||
| smethod = staticmethod(smethod) | ||
|
|
||
| def cmethod(cls, something): | ||
| """class method-to-be""" | ||
|
|
||
| cmethod = classmethod(cmethod) | ||
|
|
||
|
|
||
| class RandomNumberGenerator: | ||
| """Generate random numbers.""" | ||
|
|
||
| def limits(self): | ||
| return self.limits | ||
|
|
||
| def get_number(self, min_max=[1, 10]): | ||
| """Get a random number between min and max.""" | ||
| assert all([isinstance(i, int) for i in min_max]) | ||
| return random.randint(*min_max) | ||
|
|
||
|
|
||
| def main(options: dict = {}) -> str: | ||
| pdb.set_trace() |
There was a problem hiding this comment.
Debugger pdb is imported and set_trace() is called
The code imports the pdb module and contains a call to pdb.set_trace(). This is a debugging statement that will pause the execution of the program, waiting for interactive user input. Leaving debugger entry points in production code can lead to unexpected hangs and potential security risks.
| def tar_something(): | ||
| os.tempnam("dir1") | ||
| subprocess.Popen("/bin/chown *", shell=True) | ||
| o.system("/bin/tar xvzf *") |
There was a problem hiding this comment.
os.system is used with a command containing a wildcard *
The code calls o.system (presumably os.system), which executes a command via the shell. The command /bin/tar xvzf * contains a wildcard *, making it vulnerable to command injection. An attacker who can create files could inject malicious arguments, e.g., a file named "--use-compress-program=rm -rf /".
| def tar_something(): | ||
| os.tempnam("dir1") | ||
| subprocess.Popen("/bin/chown *", shell=True) | ||
| o.system("/bin/tar xvzf *") |
There was a problem hiding this comment.
Variable o is not defined when calling o.system()
The code attempts to call o.system(), but the variable o is not defined anywhere in the scope. This will cause a NameError at runtime, crashing the program. It's likely that os was intended instead of o. This is a critical bug that prevents the function from working.
| if ( | ||
| initial_condition | ||
| and ( | ||
| isinstance(object, int) | ||
| or isinstance(object, float) | ||
| or isinstance(object, str) | ||
| ) | ||
| and isinstance(other_obj, float) | ||
| and isinstance(foo, str) | ||
| or (isinstance(bar, float) or isinstance(bar, str)) | ||
| and (isinstance(baz, float) or isinstance(baz, int)) | ||
| ): |
There was a problem hiding this comment.
Multiple isinstance checks are used instead of a single check with a tuple
The if condition uses multiple isinstance checks with or. This can be simplified by passing a tuple of types to a single isinstance call, which is more efficient and readable. For example, isinstance(object, (int, float, str)) is preferred over three separate checks.
| def check(x): | ||
| if x == 1 or x == 2 or x == 3: | ||
| print("Yes") | ||
| elif x != 2 or x != 3: |
There was a problem hiding this comment.
The condition x != 2 or x != 3 is always true
The condition x != 2 or x != 3 will always evaluate to True for any number x. A number cannot be equal to both 2 and 3 simultaneously, so it must be not equal to at least one of them. This is a logical error and likely not the intended behavior.
| a = 1 | ||
| b = 2 | ||
| c = 3 | ||
| return a < b and b < c |
There was a problem hiding this comment.
Comparison a < b and b < c can be chained as a < b < c
The expression a < b and b < c is a valid but unpythonic way to check if b is between a and c. Python supports chained comparisons, which are more readable and efficient. The code can be simplified to a < b < c.
| @@ -0,0 +1,129 @@ | |||
| import random | |||
| import pdb | |||
There was a problem hiding this comment.
Module pdb imported at line 2 risks debug statements in production
The module pdb is imported on line 2 for debugging purposes but left in the checked-in code. This increases the risk of unintentional debugger activation in production, which can halt execution and expose internals. Remove the pdb import and all related debug calls before deployment.
| import subprocess | ||
| import abc | ||
|
|
||
| # from django.db.models.expressions import RawSQL |
There was a problem hiding this comment.
Commented import RawSQL unused in module, cluttering at import section
The import statement RawSQL from django.db.models.expressions on line 8 is commented out yet remains in the code. This commented code adds unnecessary clutter and decreases readability, potentially confusing developers about its purpose. Remove the commented lines to clean up and maintain the codebase.
| if __name__ == "__main__": | ||
| args = ["--disable", "all"] | ||
| f = open("/tmp/.deepsource.toml", "r") | ||
| f.write("config file.") |
There was a problem hiding this comment.
File object f opened in read mode but used with write() on line 122
The file f is opened in read mode ("r") on line 122 but immediately used with write(), which causes an IOError. This is a misuse of file modes leading to runtime exceptions that break program flow. Fix by opening the file with write mode ("w") to allow writing to the file as intended.
| def check(x): | ||
| if x == 1 or x == 2 or x == 3: | ||
| print("Yes") | ||
| elif x != 2 or x != 3: |
There was a problem hiding this comment.
The condition x != 2 or x != 3 is always true
The condition x != 2 or x != 3 will always evaluate to True for any integer x. If x is 2, x != 3 is true. If x is 3, x != 2 is true. If x is any other number, both are true. This is a logical error and the code block will always be executed.
|
|
||
| # from django.db.models.expressions import RawSQL | ||
|
|
||
| AWS_SECRET_KEY = "d6s$f9g!j8mg7hw?n&2" |
There was a problem hiding this comment.
Hardcoded secret AWS_SECRET_KEY found in source code
A secret key is hardcoded in the source file. This is a significant security risk, as it exposes sensitive credentials to anyone with access to the codebase. Secrets should be managed through a secure vault or environment variables, not stored in code.
|
|
||
| def tar_something(): | ||
| os.tempnam("dir1") | ||
| subprocess.Popen("/bin/chown *", shell=True) |
There was a problem hiding this comment.
subprocess.Popen is used with shell=True which is a security risk
Using subprocess.Popen with shell=True is a security risk. The command string contains a wildcard * which will be expanded by the shell. An attacker who can control filenames in the current directory could inject malicious arguments, leading to arbitrary command execution.
| def tar_something(): | ||
| os.tempnam("dir1") | ||
| subprocess.Popen("/bin/chown *", shell=True) | ||
| o.system("/bin/tar xvzf *") |
There was a problem hiding this comment.
os.system is used to execute a shell command with a wildcard
The code uses o.system (presumably os.system) to execute a shell command containing a wildcard *. This is vulnerable to command injection, as an attacker controlling filenames in the directory could execute arbitrary code. os.system is deprecated in favor of subprocess.
| a = 1 | ||
| b = 2 | ||
| c = 3 | ||
| return a < b and b < c |
There was a problem hiding this comment.
A chained comparison is written in a verbose way
The chained comparison a < b and b < c can be simplified in Python. Python supports chained comparisons, which are more readable. This can be rewritten as a < b < c, which is more concise and idiomatic.
| f.write("config file.") | ||
| f.close() | ||
| assert args is not None | ||
| for i in range(len(args)): |
There was a problem hiding this comment.
Unpythonic loop using range(len(...)) to iterate
The loop for i in range(len(args)) is unpythonic. It is more idiomatic and readable to iterate directly over the elements of the list. Instead of accessing elements by index with args[i], you can use a direct loop variable.
| f.close() | ||
| assert args is not None | ||
| for i in range(len(args)): | ||
| has_truthy = True if args[i] else False |
There was a problem hiding this comment.
Verbose boolean conversion True if ... else False is used
The expression True if args[i] else False is a verbose way to convert a value to a boolean. The more concise and idiomatic way to do this is to use the bool() constructor, i.e., bool(args[i]).
| self.limits = (1, 10) | ||
|
|
||
| def get_number(self, min_max): | ||
| raise NotImplemented |
There was a problem hiding this comment.
Variable error raised with unsupported None or object type at line 20
The variable error is assigned None initially, then potentially assigned exceptions conditionally, and finally raised on line 20. Raising a variable that can be None or not an instance of Exception causes a TypeError. Replace by raising exception objects directly within conditional blocks to avoid unsupported raises.
| self.limits = (1, 10) | ||
|
|
||
| def get_number(self, min_max): | ||
| raise NotImplemented |
There was a problem hiding this comment.
Function get_number() raises invalid exception NotImplemented at line 20
The function get_number() raises NotImplemented on line 20, which is not a valid exception type and will result in a TypeError. This causes the program to crash unexpectedly and breaks expected exception handling. Replace raise NotImplemented with raise NotImplementedError or use return NotImplemented if signalling unimplemented operation is desired.
|
|
||
| sorted(value, key=lambda k: len(k)) | ||
|
|
||
| f = open("/tmp/.deepsource.toml", "r") |
There was a problem hiding this comment.
Local variable filename shadows global filename causing confusion in file handling
The local variable filename in the read_file(filename) function shadows the global filename defined at line 1, making the global variable inaccessible within the function. This can lead to confusion and bugs when the variable usage is ambiguous. Rename the global variable using UPPER_CASE or restructure like moving globals to main() context to avoid such shadowing.
| a = 1 | ||
| b = 2 | ||
| c = 3 | ||
| return a < b and b < c |
There was a problem hiding this comment.
Expression a < b and b < c in line 117 should use chained comparison for clarity
The boolean operation a < b and b < c is used on line 117, which can be simplified to a < b < c. Chained comparisons improve code readability and make conditions more concise. Refactor the return statement to use return a < b < c for clarity and cleaner code.
| f.close() | ||
| assert args is not None | ||
| for i in range(len(args)): | ||
| has_truthy = True if args[i] else False |
There was a problem hiding this comment.
Variable has_truthy assigned via redundant if expression in loop at line 126
The variable has_truthy is assigned using True if args[i] else False on line 126, which is a redundant if expression. This reduces code readability and may confuse maintainers. Simplify assignment by using has_truthy = bool(args[i]) for cleaner, more idiomatic code.
|
|
||
|
|
||
| def tar_something(): | ||
| os.tempnam("dir1") |
There was a problem hiding this comment.
Use of insecure and deprecated function os.tempnam
The function os.tempnam is used, which is insecure and has been deprecated. It is vulnerable to a race condition where a malicious actor could create a symbolic link with the generated name before the program opens the file, potentially leading to file overwrites or other exploits. Use tempfile.mkstemp instead.
|
|
||
| def tar_something(): | ||
| os.tempnam("dir1") | ||
| subprocess.Popen("/bin/chown *", shell=True) |
There was a problem hiding this comment.
subprocess.Popen is used with shell=True and a wildcard character *
The function subprocess.Popen is called with shell=True and a command string containing a wildcard *. This can lead to command injection, as the shell will expand the wildcard. An attacker could create files with malicious names (e.g., ; rm -rf /) in the current directory to execute arbitrary commands.
| def tar_something(): | ||
| os.tempnam("dir1") | ||
| subprocess.Popen("/bin/chown *", shell=True) | ||
| o.system("/bin/tar xvzf *") |
There was a problem hiding this comment.
os.system is used with a command containing a wildcard character *
The function os.system is used with a command string containing a wildcard *. This is vulnerable to command injection. The shell expands *, allowing an attacker to execute arbitrary commands by creating files with specially crafted names in the target directory. os.system is inherently risky and should be avoided.
| def check(x): | ||
| if x == 1 or x == 2 or x == 3: | ||
| print("Yes") | ||
| elif x != 2 or x != 3: |
There was a problem hiding this comment.
The condition x != 2 or x != 3 is tautological and always evaluates to true
The condition x != 2 or x != 3 is always true for any value of x. If x is 2, x != 3 is true. If x is 3, x != 2 is true. If x is any other number, both are true. This is a logical error and the code block will always be executed if the preceding if is false.
| f.write("config file.") | ||
| f.close() | ||
| assert args is not None | ||
| for i in range(len(args)): |
There was a problem hiding this comment.
Unpythonic loop for i in range(len(args)) is used to iterate
A C-style for loop using range(len(args)) is used to iterate over a list. The Pythonic way to do this is to iterate directly over the elements of the list, which is more readable and efficient. If the index is needed, use enumerate.
| def limits(self): | ||
| return self.limits | ||
|
|
||
| def get_number(self, min_max=[1, 10]): |
There was a problem hiding this comment.
Method get_number lacks @staticmethod decorator impacting class instances
The method get_number on line 39 does not use the instance (self) but is defined as an instance method. This causes unnecessary bound method creation for every class instance, wasting memory and CPU time. Add @staticmethod decorator above get_number to optimize performance.
| elif x == 10 or x == 20 or x == 30 and x == 40: | ||
| print("Sweet!") | ||
|
|
||
| elif x == 10 or x == 20 or x == 30: |
There was a problem hiding this comment.
Variable x compared repeatedly using or in if-elif statements at line 110
The variable x is compared with multiple values separately using or operators on line 110, causing verbose and less efficient code. This can reduce performance and readability when checking membership among several values. Replace the chained equality checks with if x in (10, 20, 30, 40): to improve clarity and speed.
| f.write("config file.") | ||
| f.close() | ||
| assert args is not None | ||
| for i in range(len(args)): |
There was a problem hiding this comment.
Range(len(args)) iteration used instead of enumerate() in loop at line 125
The code uses for i in range(len(args)) at line 125 to iterate over args by index. This style is less Pythonic and decreases readability. Replace with for i, arg in enumerate(args): to directly access the element and index, improving clarity and maintaining cleaner iteration.
| raw = '"username") AS "val" FROM "auth_user" WHERE "username"="admin" --' | ||
| return User.objects.annotate(val=RawSQL(raw, [])) |
There was a problem hiding this comment.
Use of RawSQL with a string literal demonstrates a dangerous pattern
The code uses RawSQL with a hardcoded SQL string. While not directly exploitable here because the string is not from user input, this pattern is extremely dangerous and highly susceptible to SQL injection if any part of the raw string is ever constructed from external data. Use parameterized queries instead.
|
|
||
| # from django.db.models.expressions import RawSQL | ||
|
|
||
| AWS_SECRET_KEY = "d6s$f9g!j8mg7hw?n&2" |
There was a problem hiding this comment.
Hardcoded AWS_SECRET_KEY found in source code
The AWS_SECRET_KEY is hardcoded in the source file. This is a major security risk, as it exposes sensitive credentials to anyone with access to the codebase. Secrets should be loaded from a secure vault or environment variables.
| f.close() | ||
|
|
||
|
|
||
| def moon_chooser(moon, moons=["europa", "callisto", "phobos"]): |
There was a problem hiding this comment.
Function moon_chooser uses a mutable list as a default argument
The default argument moons is a list. It will be mutated by moons.append(moon) and this mutation will persist across calls to the function, leading to unexpected behavior. Default arguments should be immutable.
|
|
||
|
|
||
| def tar_something(): | ||
| os.tempnam("dir1") |
There was a problem hiding this comment.
os.tempnam is deprecated and insecure due to race conditions
os.tempnam is deprecated and vulnerable to a race condition (TOCTOU) where an attacker can create a symlink with the predicted temporary filename to a sensitive file, potentially causing it to be overwritten or modified by the application. Use the tempfile module for secure temporary file creation.
|
|
||
| def tar_something(): | ||
| os.tempnam("dir1") | ||
| subprocess.Popen("/bin/chown *", shell=True) |
There was a problem hiding this comment.
Use of shell=True with wildcard * allows command injection
The subprocess.Popen call uses shell=True with a command containing a wildcard *. An attacker who can control filenames in the current directory can inject arbitrary commands or arguments, leading to code execution. For example, a file named ; rm -rf / could be executed by the shell.
| def tar_something(): | ||
| os.tempnam("dir1") | ||
| subprocess.Popen("/bin/chown *", shell=True) | ||
| o.system("/bin/tar xvzf *") |
There was a problem hiding this comment.
Use of os.system with wildcard * allows command injection
The os.system call executes a command in a subshell, and the wildcard * is expanded by the shell. An attacker who can control filenames in the current directory can inject arbitrary commands, leading to code execution. This is also a bug since o is not defined.
| def check(x): | ||
| if x == 1 or x == 2 or x == 3: | ||
| print("Yes") | ||
| elif x != 2 or x != 3: |
There was a problem hiding this comment.
The condition x != 2 or x != 3 is always true
The condition x != 2 or x != 3 will always evaluate to True for any integer x. If x is 2, x != 3 is true. If x is 3, x != 2 is true. If x is any other number, both are true. The programmer likely intended to use and.
| else: | ||
| value = iter(value) | ||
|
|
||
| sorted(value, key=lambda k: len(k)) |
There was a problem hiding this comment.
The result of the sorted() function is not used
The sorted() function returns a new sorted list, but the result is not assigned to any variable or used in any way. The original value (which is an iterator) is not modified. This line of code has no effect on the program's execution.
| else: | ||
| value = iter(value) | ||
|
|
||
| sorted(value, key=lambda k: len(k)) |
There was a problem hiding this comment.
Lambda expression lambda k: len(k) used unnecessarily in sorted on line 57
The lambda k: len(k) is used as a key function in the sorted call on line 57 but simply wraps the built-in len function. This adds unnecessary complexity and reduces readability. Replace the lambda with the direct use of len by passing key=len to sorted, improving clarity and debugging.
| f.write("config file.") | ||
| f.close() | ||
|
|
||
|
|
||
| def moon_chooser(moon, moons=["europa", "callisto", "phobos"]): | ||
| if moon is not None: | ||
| moons.append(moon) | ||
|
|
||
| return random.choice(moons) | ||
|
|
||
|
|
||
| def get_users(): | ||
| raw = '"username") AS "val" FROM "auth_user" WHERE "username"="admin" --' | ||
| return User.objects.annotate(val=RawSQL(raw, [])) | ||
|
|
||
|
|
||
| def tar_something(): | ||
| os.tempnam("dir1") | ||
| subprocess.Popen("/bin/chown *", shell=True) | ||
| o.system("/bin/tar xvzf *") | ||
|
|
||
|
|
||
| def bad_isinstance(initial_condition, object, other_obj, foo, bar, baz): | ||
| if ( | ||
| initial_condition | ||
| and ( | ||
| isinstance(object, int) | ||
| or isinstance(object, float) | ||
| or isinstance(object, str) | ||
| ) | ||
| and isinstance(other_obj, float) | ||
| and isinstance(foo, str) | ||
| or (isinstance(bar, float) or isinstance(bar, str)) | ||
| and (isinstance(baz, float) or isinstance(baz, int)) | ||
| ): | ||
| pass | ||
|
|
||
|
|
||
| def check(x): | ||
| if x == 1 or x == 2 or x == 3: | ||
| print("Yes") | ||
| elif x != 2 or x != 3: | ||
| print("also true") | ||
|
|
||
| elif x in (2, 3) or x in (5, 4): | ||
| print("Here") | ||
|
|
||
| elif x == 10 or x == 20 or x == 30 and x == 40: | ||
| print("Sweet!") | ||
|
|
||
| elif x == 10 or x == 20 or x == 30: | ||
| print("Why even?") | ||
|
|
||
| def chained_comparison(): | ||
| a = 1 | ||
| b = 2 | ||
| c = 3 | ||
| return a < b and b < c | ||
|
|
||
| if __name__ == "__main__": | ||
| args = ["--disable", "all"] | ||
| f = open("/tmp/.deepsource.toml", "r") | ||
| f.write("config file.") |
There was a problem hiding this comment.
Attempting to write to a file opened in read mode
The code attempts to write to a file that was opened in read-only mode ("r"). This will raise an io.UnsupportedOperation: not writable exception at runtime, causing the program to crash. This error occurs in both the main function and the __main__ block.
|
|
||
| def get_number(self, min_max=[1, 10]): | ||
| """Get a random number between min and max.""" | ||
| assert all([isinstance(i, int) for i in min_max]) |
There was a problem hiding this comment.
Assert statement assert all([isinstance(i, int) for i in min_max]) in get_number() risks removal on optimization
The assert statement on line 41 in the get_number() method performs an internal check for integer types but is removed when Python code is run with optimization flags like -O. This leads to potential silent failures or unexpected behavior in production. Replace the assert with an explicit conditional check and raise an appropriate exception to ensure the check is always enforced.
| elif x in (2, 3) or x in (5, 4): | ||
| print("Here") | ||
|
|
||
| elif x == 10 or x == 20 or x == 30 and x == 40: |
There was a problem hiding this comment.
Condition mixes or and and operators without parentheses, causing ambiguity
The condition mixes or and and operators without explicit parentheses. Due to operator precedence (and is evaluated before or), the logic is (x == 10) or (x == 20) or (x == 30 and x == 40). This is confusing and likely not the intended logic. Use parentheses to clarify intent.
|
|
||
|
|
||
| def main(options: dict = {}) -> str: | ||
| pdb.set_trace() |
There was a problem hiding this comment.
Use of pdb.set_trace() causes unintended program halts
The code calls pdb.set_trace() on line 46, which triggers a breakpoint stopping execution and entering debug mode unexpectedly. This disrupts normal program flow and may expose debugging internals or stall production applications.
Remove or comment out all pdb.set_trace() calls before committing code to prevent unintended debug halts in production environments.
| f.close() | ||
|
|
||
|
|
||
| def moon_chooser(moon, moons=["europa", "callisto", "phobos"]): |
There was a problem hiding this comment.
Mutable default moons list causes state persistence across calls
The function moon_chooser uses a mutable default argument moons initialized to a list. This list is shared across all calls, so appending moon modifies it permanently, causing unexpected side effects and bugs in subsequent calls.
Replace the default list with None and initialize inside the function: use moons=None, then if moons is None: moons = [] to ensure a fresh list on every call.
|
|
||
|
|
||
| def tar_something(): | ||
| os.tempnam("dir1") |
There was a problem hiding this comment.
Use of os.tempnam() allows symlink attacks in file creation
The os.tempnam() function creates temporary file names insecurely, enabling attackers to exploit symlink redirection vulnerabilities. This can lead to unauthorized file access or modification during tar_something() execution.
Replace os.tempnam() with safer alternatives like tempfile.NamedTemporaryFile() or use os.tmpfile() for secure, atomic temporary file creation.
1 participant
Implement a random number generator and various utility functions.