3
\$\begingroup\$

This form will store research article information in a database. It would be nice to somehow add buttons to add additional author and university fields or also multiple universities for 1 author. Do you see any glaring issues with my code?

Screenshot of current page

<!DOCTYPE html>
<html lang="en">
<head>
  <title>Input Form - Research Ranker</title>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <link rel="stylesheet" href="http://maxcdn.bootstrapcdn.com/bootstrap/3.3.5/css/bootstrap.min.css">
  <link rel="shortcut icon" href="/../img/favicon.ico">
  <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.3/jquery.min.js"></script>
  <script src="http://maxcdn.bootstrapcdn.com/bootstrap/3.3.5/js/bootstrap.min.js"></script>
  <link rel="stylesheet" href="//code.jquery.com/ui/1.11.4/themes/smoothness/jquery-ui.css">
  <script src="//code.jquery.com/jquery-1.10.2.js"></script>
  <script src="//code.jquery.com/ui/1.11.4/jquery-ui.js"></script>
   <script>
  $(function() {
    $( "#article_name" ).autocomplete({
      source: 'articlesearch.php'
    });
    $( "#author1_name" ).autocomplete({
      source: 'authorsearch.php'
    });
    $( "#author2_name" ).autocomplete({
      source: 'authorsearch.php'
    });
    $( "#author3_name" ).autocomplete({
      source: 'authorsearch.php'
    });
    $( "#author4_name" ).autocomplete({
      source: 'authorsearch.php'
    });
    $( "#author5_name" ).autocomplete({
      source: 'authorsearch.php'
    });
    $( "#author6_name" ).autocomplete({
      source: 'authorsearch.php'
    });
    $( "#author1_university" ).autocomplete({
      source: 'universitysearch.php'
    });
    $( "#author2_university" ).autocomplete({
      source: 'universitysearch.php'
    });
    $( "#author3_university" ).autocomplete({
      source: 'universitysearch.php'
    });
    $( "#author4_university" ).autocomplete({
      source: 'universitysearch.php'
    });
    $( "#author5_university" ).autocomplete({
      source: 'universitysearch.php'
    });
    $( "#author6_university" ).autocomplete({
      source: 'universitysearch.php'
    });
  });
  </script>
</head>
<body>



<?PHP
 
if(!isset($_POST['submit'])){
    print '
	<div class="container-fluid ui-widget">


	<form class="form-horizontal" method="post" action="articleinput2.php" onsubmit="return confirm("Is all of your data correct?");">
	<fieldset>

	<!-- Article Input -->
	<legend>Article Input</legend>

	<!-- Journal Radio Buttons (value is journal ISSN)-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="journal_radio">Journal</label>
	  <div class="col-md-4">
	  <div class="radio">
		<label for="journal_radio-0">
		  <input type="radio" name="journal_radio" id="journal_radio-0" value="2162-9730" checked="checked">
		  MIS Quarterly
		</label>
	  </div>
	  <div class="radio">
		<label for="journal_radio-1">
		  <input type="radio" name="journal_radio" id="journal_radio-1" value="1047-7047">
		  Information Systems Research
		</label>
	  </div>
	  <div class="radio">
		<label for="journal_radio-2">
		  <input type="radio" name="journal_radio" id="journal_radio-2" value="0742-1222">
		  Journal of Management Information Systems
		</label>
	  </div>
	  <div class="radio">
		<label for="journal_radio-3">
		  <input type="radio" name="journal_radio" id="journal_radio-3" value="1536-9323">
		  Journal of the Association for Information Systems
		</label>
	  </div>
	  <div class="radio">
		<label for="journal_radio-4">
		  <input type="radio" name="journal_radio" id="journal_radio-4" value="1476-9344">
		  European Journal of Information Systems
		</label>
	  </div>
	  <div class="radio">
		<label for="journal_radio-5">
		  <input type="radio" name="journal_radio" id="journal_radio-5" value="1365-2575">
		  Information Systems Journal
		</label>
	  </div>
	  <div class="radio">
		<label for="journal_radio-6">
		  <input type="radio" name="journal_radio" id="journal_radio-6" value="0963-8687">
		  Journal of Strategic Information Systems
		</label>
	  </div>
	  <div class="radio">
		<label for="journal_radio-7">
		  <input type="radio" name="journal_radio" id="journal_radio-7" value="0268-3962">
		  Journal of Information Technology
		</label>
	  </div>
	  <div class="radio">
		<label for="journal_radio-8">
		  <input type="radio" name="journal_radio" id="journal_radio-8" value="0167-9236">
		  Decision Support Systems
		</label>
	  </div>
	  <div class="radio">
		<label for="journal_radio-9">
		  <input type="radio" name="journal_radio" id="journal_radio-9" value="0378-7206">
		  Information and Management
		</label>
	  </div>
	  </div>
	</div>

	<!-- Article Name Input-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="article_name">Article Name:</label>  
	  <div class="col-md-5">
	  <input id="article_name" name="article_name" type="text" placeholder="Article Name" class="form-control input-md" required="" maxlength = "60">
		
	  </div>
	</div>

	<!-- Author 1-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="author1_name">Author 1:</label>  
	  <div class="col-md-4">
	  <input id="author1_name" name="author1_name" type="text" placeholder="Author Name" class="form-control input-md" required="" maxlength="40">
		
	  </div>
	</div>

	<!-- Author 1 University -->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="author1_university">University:</label>  
	  <div class="col-md-4">
	  <input id="author1_university" name="author1_university" type="text" placeholder="Author University" class="form-control input-md" required="" maxlength="55">
		
	  </div>
	</div>

	<!-- Author 2-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="author2_name">Author 2:</label>  
	  <div class="col-md-4">
	  <input id="author2_name" name="author2_name" type="text" placeholder="" class="form-control input-md" maxlength="40">
		
	  </div>
	</div>

	<!-- Author 2 University-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="author2_university">University:</label>  
	  <div class="col-md-4">
	  <input id="author2_university" name="author2_university" type="text" placeholder="" class="form-control input-md" maxlength="55">
		
	  </div>
	</div>

	<!-- Author 3-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="author3_name">Author 3:</label>  
	  <div class="col-md-4">
	  <input id="author3_name" name="author3_name" type="text" placeholder="" class="form-control input-md" maxlength="40">
		
	  </div>
	</div>

	<!-- Author 3 University-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="author3_university">University:</label>  
	  <div class="col-md-4">
	  <input id="author3_university" name="author3_university" type="text" placeholder="" class="form-control input-md" maxlength="55">
		
	  </div>
	</div>

	<!-- Author 4-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="author4_name">Author 4: </label>  
	  <div class="col-md-4">
	  <input id="author4_name" name="author4_name" type="text" placeholder="" class="form-control input-md" maxlength="40">
		
	  </div>
	</div>

	<!-- Author 4 University-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="author4_university">University:</label>  
	  <div class="col-md-4">
	  <input id="author4_university" name="author4_university" type="text" placeholder="" class="form-control input-md" maxlength="55">
		
	  </div>
	</div>

	<!-- Author 5-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="author5_name">Author 5:</label>  
	  <div class="col-md-4">
	  <input id="author5_name" name="author5_name" type="text" placeholder="" class="form-control input-md" maxlength="40">
		
	  </div>
	</div>

	<!-- Author 5 University-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="author5_university">University:</label>  
	  <div class="col-md-4">
	  <input id="author5_university" name="author5_university" type="text" placeholder="" class="form-control input-md" maxlength="55">
		
	  </div>
	</div>

	<!-- Author 6-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="author6_name">Author 6:</label>  
	  <div class="col-md-4">
	  <input id="author6_name" name="author6_name" type="text" placeholder="" class="form-control input-md" maxlength="40">
		
	  </div>
	</div>

	<!-- Author 6 University-->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="author6_university">University:</label>  
	  <div class="col-md-4">
	  <input id="author6_university" name="author6_university" type="text" placeholder="" class="form-control input-md" maxlength="55">
		
	  </div>
	</div>

	<!-- Select Year Published -->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="year">Year Published</label>
	  <div class="col-md-4">
		<select id="year" name="year" class="form-control">
		  <option value="2010">2010</option>
		  <option value="2011">2011</option>
		  <option value="2012">2012</option>
		  <option value="2013">2013</option>
		  <option value="2014">2014</option>
		  <option value="2015">2015</option>
		</select>
	  </div>
	</div>

	<!-- Is Security Checkbox -->
	<div class="form-group">
	  <label class="col-md-4 control-label" for="is_security"></label>
	  <div class="col-md-4">
		<label class="checkbox-inline" for="is_security">
		  <input type="checkbox" name="is_security" id="is_security" value="1">
		  Security Related
		</label>
	  </div>
	</div>

	<!-- Submit Form Button -->
	<div class="form-group">
	  <div class="col-md-4 control-label">
		<button id="submit" name="submit" class="btn btn-primary">Submit</button>
	  </div>
	</div>

	</fieldset>
	</form>


	</div>';
	
	} else {
		
		$dbHost = "localhost";
		$dbUsername = "admin";
		$dbPassword = "";
		$dbName = "webdb";

		// Create connection
		$db = new mysqli($dbHost, $dbUsername, $dbPassword, $dbName);
		// Check connection
		if ($db->connect_error) {
		die("Connection failed: " . $db->connect_error);
		} else {
		// Post PHP variables
		$journal = $_POST['journal_radio'];
		$article_name = $_POST['article_name'];
		$author1 = $_POST['author1_name'];
		$author1uni = $_POST['author1_university'];
		$author2 = $_POST['author2_name'];
		$author2uni = $_POST['author2_university'];
		$author3 = $_POST['author3_name'];
		$author3uni = $_POST['author3_university'];
		$author4 = $_POST['author4_name'];
		$author4uni = $_POST['author4_university'];
		$author5 = $_POST['author5_name'];
		$author5uni = $_POST['author5_university'];
		$author6 = $_POST['author6_name'];
		$author6uni = $_POST['author6_university'];
		$year = $_POST['year'];
		$security = $_POST['is_security'];
		function authorinfoinsert($author, $number, $authoruni, $articleid, $article, $db){
				//check if Author already exists
				$authorquery = $db->query("SELECT Author_Name FROM Authors WHERE Author_Name = '$author'");
				if($authorquery->num_rows != 0) {
				   echo '<a href=""> New Input </a><br />';
				   echo 'Author ' . $number . ' - ' . $author . ' - already present<br />';
			
				} else {
					//insert author if doesnt exist
					$authorinsert = "INSERT INTO Authors (Author_Name) VALUES ('$author')";
				
					if ($db->query($authorinsert) === TRUE) {
					echo 'Author ' . $number . ' - ' . $author . ' - created successfully.<br />';
					} else {
					echo 'Error: ' . $authorinsert . '<br />' . $db->error . '<br />';
					}
				  }
				  
				// store Author Author_ID value
				
				
				$result = $db->query("SELECT Author_ID FROM Authors WHERE Author_Name = '$author'");
				$row = $result->fetch_assoc();
				$authorid = $row['Author_ID'];
				
				
				//check if Author University exists
				$authoruniquery = $db->query("SELECT University_Name FROM University WHERE University_Name = '$authoruni'");
				
				if($authoruniquery->num_rows != 0) {
				   echo '<a href=""> New Input </a><br />';
				   echo 'Author ' . $number . ' University already present <br />';
			
				} else {
					//insert university if doesnt exist
					$uniinsert = "INSERT INTO University (University_Name) VALUES ('$authoruni')";
				
					if ($db->query($uniinsert) === TRUE) {
					echo 'Author ' . $number . ' University - ' . $authoruni . ' - created successfully.<br />';
					} else {
					echo 'Error: ' . $uniinsert . "<br>" . $db->error . '<br />';
					}
				  }
				
				// store Author University_ID value

				$result = $db->query("SELECT University_ID FROM University WHERE University_Name = '$authoruni'");
				$row = $result->fetch_assoc();
				$authoruniid = $row['University_ID'];
				
				
				//Enter Author and Article in Articles_Authored
				$authoredquery = $db->query("SELECT Author_ID, Article_ID FROM Articles_Authored WHERE Author_ID = '$authorid' AND Article_ID ='$articleid'");
				if($authoredquery->num_rows != 0) {
				   echo '<a href=""> New Input </a><br />';
				   echo 'Article Authored entry for Author ' . $number . ' - ' . $author . ' - already present<br />';
			
				} else {
				$articlesauthoredinsert = "INSERT INTO Articles_Authored (Article_ID, Author_ID, Name_Authored_As) VALUES ('$articleid','$authorid','$author')";
				
					if ($db->query($articlesauthoredinsert) === TRUE) {
					echo 'Article Authored - Author ' . $number . ' - created successfully <br />';
					} else {
					echo 'Error: ' . $articlesauthoredinsert . "<br>" . $db->error . '<br />';
					}
				}
				
				//Enter Author University instance in Author_University
				$authoruniinsert = "INSERT INTO Author_University (Article_ID, Author_ID, University_ID) VALUES ('$articleid','$authorid','$authoruniid')";
				
					if ($db->query($authoruniinsert) === TRUE) {
					echo 'University instance for Author ' . $number  . ' created successfully<br />';
					} else {
					echo 'Error: ' . $authoruniinsert . "<br>" . $db->error . '<br />';
					}
				}

				
				
		  //check if article exist
		  $articlequery = $db->query("SELECT Article_Title, ISSN FROM Articles WHERE Article_Title = '$article_name' AND ISSN = '$journal'");
		  if($articlequery->num_rows != 0) {
			   echo '<a href=""> New Input </a><br />';
			   echo "<script type= 'text/javascript'>alert('Article Already Present');</script>";
		
		  } else {

				//insert article if doesn't exist	
				$articleinsert = "INSERT INTO Articles (ISSN, Year_Published, Is_Security, Article_Title)
				VALUES ('$journal','$year','$security','$article_name')";

					if ($db->query($articleinsert) === TRUE) {
					echo 'New Article - ' . $article_name . ' - created successfully<br />';
					} else {
					echo 'Error: ' . $articleinsert . "<br>" . $db->error . '<br />';
					}
				// store Article_ID value
				
				$result = $db->query("SELECT Article_ID FROM Articles WHERE Article_Title = '$article_name' AND ISSN = '$journal'");
				$row = $result->fetch_assoc();
				$articleid = $row['Article_ID'];
				
				//insert author 1 info
				authorinfoinsert($author1, 1, $author1uni, $articleid, $article, $db);
				//check if Author 2 has input
				if (!empty($author2)){
					authorinfoinsert($author2, 2, $author2uni, $articleid, $article, $db);				
				}
				
				//check if Author 3 has input
				if (!empty($author3)){
					authorinfoinsert($author3, 3, $author3uni, $articleid, $article, $db);			
				}
				//check if Author 4 has input
				if (!empty($author4)){
					authorinfoinsert($author4, 4, $author4uni, $articleid, $article, $db);			
				}
				
				//check if Author 5 has input
				if (!empty($author5)){
					authorinfoinsert($author5, 5, $author5uni, $articleid, $article, $db);			
				}

				//check if Author 6 has input
				if (!empty($author6)){
					authorinfoinsert($author6, 6, $author6uni, $articleid, $article, $db);			
				} else{
					
					$db->close();
					
				}
			}
		}
	}
?>


</body>
</html>

\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review! Sure we can review for glaring issues. However implementing features that you describe as "it would be nice" is not within our scope. I hope you'll get good answers! \$\endgroup\$
    – janos
    Commented Nov 23, 2015 at 8:29
  • \$\begingroup\$ Are you looking to make this form dynamic in terms of author and university ? for example if u add one author in form using Jquery or something then u want server side validation for newly created field and then save to the DB? \$\endgroup\$
    – kaiyum2012
    Commented Nov 23, 2015 at 11:22

1 Answer 1

1
\$\begingroup\$

SQL Injection

You are completely open to SQL injection in all your queries. This means that an attacker can read any data they want from the database, and possibly read files, create new files, or execute commands.

You should never put any user input (this may be from GET, POST, the database itself, anywhere really) into a query. You need to use prepared statements instead.

XSS

You are also vulnerable to XSS. One example is echo 'New Article - ' . $article_name, but really all echoes that contain user input are vulnerable.

This means that an attacker can inject JavaScript into the context of a victim, making it for example possible to steal cookies, change the websites content (eg for phishing attacks), bypass CSRF protection, etc.

You need to HTML encode all user input before echoing it.

CSRF

You are also vulnerable to CSRF. This means that an attacker can perform any action for an already authenticated user, if that user visits an attacker controlled webpage.

Formatting

Your indentation is off, making your code hard to read. For example, I have no idea where authorinfoinsert ends.

You should also store functions in their own file to increase reusability. If you include a file containing one or multiple functions, you don't want any side effects.

But even if you need to store the function in this file, don't put it inside an else case in the middle of your code, put it either at the beginning or at the end.

And you should use camelCase or snake_case for your variable names. Something like articlesauthoredinsert is hard to read.

Structure

Your HTML code contains quite a bit of duplication. You could create functions such as getAuthorInput($authorNumber), or you could use a proper templating engine.

Your code is also too long.

Your function authorinfoinsert for example does too much: It prints stuff, it checks the existence of stuff, it inserts the author info, and it inserts a bunch of other stuff. I would create separate functions for each of these to increase readability.

Misc

  • Your error handling doesn't seem ideal. For example, if you could not insert a new article, you inform the user (good), but then you just keep going and try to insert the author anyways, which doesn't seem to make sense.
  • you only close the db if the last author is empty, which seems odd.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.