I want to add a book to the database, but it is not added and the file is not uploaded. It does not show any error
This code is largely based on the bad programming examples found at w3schools, as a result it contains a lot of unnecessary typing, misleading errors, when it does display errors, and in this case no errors.
Here’s a laundry list of things the code should not or should do instead -
- The post method form processing code should be above the start of the html document.
- The logincheck code is likely insecure and allows access to the rest of the code on the page even if the user is not logged in.
- Don’t write out discrete variables for every field and every error. Instead, simply use arrays for these and operate on the form data as a set and put any user/validation errors in to an array using the field name as the array index.
- Don’t try to detect if the submit button is set. There are cases where it won’t be. Instead, detect if a post method form has been submitted.
- If the total size of the form data exceeds the post_max_size setting, both the $_POST and $_FILES arrays will be empty. After you do item #4, you must test for this condition and setup an error message for the user that the form data was too large and could not be processed. After you have determined that there is $_POST and $_FILES data, you can continue to process the form data.
- For the $_POST data, forget you ever saw this test_input() function and delete it. It is misnamed and the only thing it is doing properly is trimming the value.
- After you do item #3, you can trim all the post input data at once, using a single line of code. You should trim all the input data before validating it. The current logic is testing if the raw input data is empty(), then trimming it and further validating it. This will produce misleading results if all white-space characters are entered for a value.
- There’s a typo in the ‘author’ error code. There’s two $$ at the start of a variable name.
- Using preg_match for validating data is a good idea, however be careful about what you are preventing. Things like the book name, author, and price can contain more than just letters and numeric digits. The name and author can contain spaces and some punctuation characters, and the price can contain a ‘.’.
- $_FILES[‘book_img’] not being set isn’t the condition for a file not being selected. Except for the case in item #5, $_FILES[‘book_img’] will be set once the form has been submitted. The $_FILES[‘book_img’][‘error’] element will be a 4 (UPLOAD_ERR_NO_FILE) if no file was selected. You must test the [‘error’] element first, before using any of the uploaded file information. The [‘error’] element will be a 0 (UPLOAD_ERR_OK) if the file was successfully uploaded. You should actually setup a unique and helpful error message for each possible upload error number. These errors can be found in the php.net documentation.
- If the file was successfully uploaded, you can then validate the file information.
- The [‘type’] element comes from the submitted form data and cannot be trusted. You should determine the mime type of the file on the server. Use either mime_content_type() or finfo_file() to find the mime type of a file.
- You should use separate error elements for the different file validation tests and use a separate array to hold the file validation errors from the post data validation errors. The current code uses the same error element for all the file validation errors and will only report the last error found, not all the errors found.
- DRY (Don’t Repeat Yourself.) Don’t repeat logic, e.g. the retesting of upload validation. You also have a logic mistake in the existing repeated code in that a file that’s exactly the same size as $maxsize won’t produce an error nor will it be processed. Instead test if there are no file validation errors.
- Computers don’t do random things very well. To make a unique filename for the uploaded file, it is simpler to just insert the data record, get the last insert id from the query, and use the last insert id as part of the filename.
- After the end of all the validation logic, just test if the arrays holding the post/file user/validation errors are empty.
- You should ALWAYS list out the columns you are suppling values for in an INSERT query. This will help prevent mistakes. Also, don’t include the autoincrement column/value in the query.
- You should use a prepared query when supplying external, unknown, dynamic values to a query when it gets executed. This would also be a good time to switch to the much simpler and more modern PDO database extension.
- If the INSERT query can result in duplicate data, the unique column(s) must be defined as a unique index, and you must have error handling for this query to detect if a duplicate index error has occurred.
- After all the form processing logic, if there are no errors, redirect to the exact same url of the current page. This will cause a get request for the page, which will prevent the browser from trying to resubmit the form data.
- To display a one-time success message, store it in a session variable, then test, display, and clear the session variable at the appropriate location in the html document.
- Repopulate the post fields with any existing values so that the user doesn’t need to keep reentering the values upon an error.
- Any dynamic value you output in a html context need to have htmlentities() applied to it to help prevent cross site scripting.
Here’s an additional point - you should have a category database table, then use a select/option menu to pick from an existing category, then use the category_id as the value in the insert query.
thanks so much**********************