Merge request discussions API doesn't reject an error input in some case

Summary

Merge request discussions API (POST /api/v4/projects/1/merge_requests/2/discussions) passes error input in some cases.
API returns 500 but comment is saved to DB.
This issue also leads to corruption of notes data.

Steps to reproduce

Case 1

Make some changes to README.md and commit & push. (Call this commit-1)
Create Merge Request.
Make another changes and commit & push. (Call this commit-2)
Call curl --request POST --header "PRIVATE-TOKEN: xxxx" 'https://(host)/api/v4/projects/1/merge_requests/1/discussions?body=comment&position\[base_sha\]=(base sha)&position\[start_sha\]=(start sha)&position\[position_type\]=text&position\[head_sha\]=(commit-1 sha)&position\[new_path\]=README.md&position\[new_line\]=1'

In this case, both commit-1 and commit-2 must have no changes on README.md line 1.

Case 2

Move README.md to README.md.bak and commit & push.
Call curl --request POST --header "PRIVATE-TOKEN: xxxx" 'https://(host)/api/v4/projects/1/merge_requests/1/discussions?body=comment&position\[position_type\]=text&position\[base_sha\]=(base sha)&position\[start_sha\]=(start sha)&position\[head_sha\]=(commit-1 sha)&position\[new_path\]=README1.md&position\[new_line\]=1'

Example Project

oboenikui/merge-request-api-bug-poc!1
oboenikui/merge-request-api-bug-poc!2

What is the current bug behavior?

API call is malformed, but the comment is saved to DB.
Merge Request page tries to get commented file, but can't.

What is the expected correct behavior?

API call is malformed, so API should return 400 on validate phase.

Relevant logs and/or screenshots

client logs

$ curl --request POST --header "PRIVATE-TOKEN: xxxx" 'http://127.0.0.1:8080/api/v4/projects/1/merge_requests/1/discussions?body=foo&position\[base_sha\]=a021a16e30a4452d84fc6d6e10c56d614800689f&position\[start_sha\]=a021a16e30a4452d84fc6d6e10c56d614800689f&position\[head_sha\]=144a4550c99c532046b2c87ea51bb07d6d6f6f35&position\[new_path\]=README.md&position\[position_type\]=text&position\[new_line\]=1'
{"message":"500 Internal Server Error"}⏎

server logs

2019-04-09_06:08:02.26754 2019-04-09T06:08:02.267Z 821 TID-orvlk7i5d WARN: {"context":"Job raised exception","job":{"class":"CreateNoteDiffFileWorker","args":[13],"retry":3,"queue":"create_note_diff_file","jid":"cbfdd524f615fd5dde229833","created_at":1554789925.4576952,"correlation_id":"UPtwqu9taNa","enqueued_at":1554790081.931983,"error_message":"undefined method `index' for nil:NilClass","error_class":"NoMethodError","failed_at":1554789925.8735104,"retry_count":2,"retried_at":1554790004.536265},"jobstr":"{\"class\":\"CreateNoteDiffFileWorker\",\"args\":[13],\"retry\":3,\"queue\":\"create_note_diff_file\",\"jid\":\"cbfdd524f615fd5dde229833\",\"created_at\":1554789925.4576952,\"correlation_id\":\"UPtwqu9taNa\",\"enqueued_at\":1554790081.931983,\"error_message\":\"undefined method `index' for nil:NilClass\",\"error_class\":\"NoMethodError\",\"failed_at\":1554789925.8735104,\"retry_count\":2,\"retried_at\":1554790004.536265}"}
2019-04-09_06:08:02.26786 2019-04-09T06:08:02.267Z 821 TID-orvlk7i5d WARN: NoMethodError: undefined method `index' for nil:NilClass
2019-04-09_06:08:02.26888 2019-04-09T06:08:02.267Z 821 TID-orvlk7i5d WARN: /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/diff/file.rb:103:in `diff_hunk'
2019-04-09_06:08:02.26941 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/metrics/instrumentation.rb:161:in `block in diff_hunk'
2019-04-09_06:08:02.26970 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/metrics/method_call.rb:36:in `measure'
2019-04-09_06:08:02.26998 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/metrics/instrumentation.rb:161:in `diff_hunk'
2019-04-09_06:08:02.27066 /opt/gitlab/embedded/service/gitlab-rails/app/models/diff_note.rb:39:in `create_diff_file'
2019-04-09_06:08:02.27131 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/metrics/instrumentation.rb:161:in `block in create_diff_file'
2019-04-09_06:08:02.27162 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/metrics/method_call.rb:36:in `measure'
2019-04-09_06:08:02.27191 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/metrics/instrumentation.rb:161:in `create_diff_file'
2019-04-09_06:08:02.27251 /opt/gitlab/embedded/service/gitlab-rails/app/workers/create_note_diff_file_worker.rb:9:in `perform'
2019-04-09_06:08:02.27298 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:185:in `execute_job'
2019-04-09_06:08:02.27358 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:167:in `block (2 levels) in process'
2019-04-09_06:08:02.27415 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:128:in `block in invoke'
2019-04-09_06:08:02.27464 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/metrics/sidekiq_middleware.rb:15:in `block in call'
2019-04-09_06:08:02.27508 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/metrics/transaction.rb:55:in `run'
2019-04-09_06:08:02.27551 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/metrics/sidekiq_middleware.rb:15:in `call'
2019-04-09_06:08:02.27589 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:130:in `block in invoke'
2019-04-09_06:08:02.27641 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/sidekiq_status/server_middleware.rb:7:in `call'
2019-04-09_06:08:02.27685 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:130:in `block in invoke'
2019-04-09_06:08:02.27728 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/sidekiq_middleware/correlation_logger.rb:10:in `block in call'
2019-04-09_06:08:02.27771 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/correlation_id.rb:15:in `use_id'
2019-04-09_06:08:02.27829 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/sidekiq_middleware/correlation_logger.rb:9:in `call'
2019-04-09_06:08:02.27876 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:130:in `block in invoke'
2019-04-09_06:08:02.27927 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/sidekiq_middleware/batch_loader.rb:7:in `call'
2019-04-09_06:08:02.27971 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:130:in `block in invoke'
2019-04-09_06:08:02.28014 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/sidekiq_middleware/request_store_middleware.rb:8:in `call'
2019-04-09_06:08:02.28047 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:130:in `block in invoke'
2019-04-09_06:08:02.28090 /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/sidekiq_middleware/memory_killer.rb:18:in `call'
2019-04-09_06:08:02.28134 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:130:in `block in invoke'
2019-04-09_06:08:02.28345 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sentry-raven-2.7.4/lib/raven/integrations/sidekiq.rb:9:in `call'
2019-04-09_06:08:02.28427 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:130:in `block in invoke'
2019-04-09_06:08:02.28470 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/middleware/chain.rb:133:in `invoke'
2019-04-09_06:08:02.28521 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:166:in `block in process'
2019-04-09_06:08:02.28573 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:137:in `block (6 levels) in dispatch'
2019-04-09_06:08:02.28617 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/job_retry.rb:108:in `local'
2019-04-09_06:08:02.28700 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:136:in `block (5 levels) in dispatch'
2019-04-09_06:08:02.28749 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/rails.rb:43:in `block in call'
2019-04-09_06:08:02.28793 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/activesupport-5.0.7.1/lib/active_support/execution_wrapper.rb:85:in `wrap'
2019-04-09_06:08:02.28838 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/activesupport-5.0.7.1/lib/active_support/reloader.rb:68:in `block in wrap'
2019-04-09_06:08:02.28884 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/activesupport-5.0.7.1/lib/active_support/execution_wrapper.rb:85:in `wrap'
2019-04-09_06:08:02.28928 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/activesupport-5.0.7.1/lib/active_support/reloader.rb:67:in `wrap'
2019-04-09_06:08:02.28972 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/rails.rb:42:in `call'
2019-04-09_06:08:02.29014 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:132:in `block (4 levels) in dispatch'
2019-04-09_06:08:02.29057 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:243:in `stats'
2019-04-09_06:08:02.29099 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:127:in `block (3 levels) in dispatch'
2019-04-09_06:08:02.29142 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/job_logger.rb:8:in `call'
2019-04-09_06:08:02.29252 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:126:in `block (2 levels) in dispatch'
2019-04-09_06:08:02.29299 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/job_retry.rb:73:in `global'
2019-04-09_06:08:02.29346 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:125:in `block in dispatch'
2019-04-09_06:08:02.29396 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/logging.rb:48:in `with_context'
2019-04-09_06:08:02.29438 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/logging.rb:42:in `with_job_hash_context'
2019-04-09_06:08:02.29482 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:124:in `dispatch'
2019-04-09_06:08:02.29526 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:165:in `process'
2019-04-09_06:08:02.29569 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:83:in `process_one'
2019-04-09_06:08:02.29603 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/processor.rb:71:in `run'
2019-04-09_06:08:02.29652 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/util.rb:16:in `watchdog'
2019-04-09_06:08:02.29695 /opt/gitlab/embedded/lib/ruby/gems/2.5.0/gems/sidekiq-5.2.5/lib/sidekiq/util.rb:25:in `block in safe_thread'

Output of checks

This bug happens on GitLab.com

Results of GitLab environment info

Expand for output related to GitLab environment info

System information System: Current User: git Using RVM: no Ruby Version: 2.5.3p105 Gem Version: 2.7.6 Bundler Version:1.16.6 Rake Version: 12.3.2 Redis Version: 3.2.12 Git Version: 2.18.1 Sidekiq Version:5.2.5 Go Version: unknown

GitLab information Version: 11.9.6 Revision: 14bac95 Directory: /opt/gitlab/embedded/service/gitlab-rails DB Adapter: postgresql URL: http://172.30.0.2 HTTP Clone URL: http://172.30.0.2/some-group/some-project.git SSH Clone URL: git@172.30.0.2:some-group/some-project.git Using LDAP: no Using Omniauth: yes Omniauth Providers:

GitLab Shell Version: 8.7.1 Repository storage paths:

default: /var/opt/gitlab/git-data/repositories GitLab Shell path: /opt/gitlab/embedded/service/gitlab-shell Git: /opt/gitlab/embedded/bin/git

Results of GitLab application Check

Expand for output related to the GitLab application check

GitLab Shell Version: 8.7.1 Repository storage paths:

default: /var/opt/gitlab/git-data/repositories GitLab Shell path: /opt/gitlab/embedded/service/gitlab-shell Git: /opt/gitlab/embedded/bin/git root@a8aafdb8d4fb:/# gitlab-rake gitlab:check SANITIZE=true Checking GitLab subtasks ...

Checking GitLab Shell ...

GitLab Shell: ... GitLab Shell version >= 8.7.1 ? ... OK (8.7.1) Running /opt/gitlab/embedded/service/gitlab-shell/bin/check Check GitLab API access: OK Redis available via internal API: OK

Access to /var/opt/gitlab/.ssh/authorized_keys: OK gitlab-shell self-check successful

Checking GitLab Shell ... Finished

Checking Gitaly ...

Gitaly: ... default ... OK

Checking Gitaly ... Finished

Checking Sidekiq ...

Sidekiq: ... Running? ... yes Number of Sidekiq processes ... 1

Checking Sidekiq ... Finished

Checking Incoming Email ...

Incoming Email: ... Reply by email is disabled in config/gitlab.yml

Checking Incoming Email ... Finished

Checking LDAP ...

LDAP: ... LDAP is disabled in config/gitlab.yml

Checking LDAP ... Finished

Checking GitLab App ...

Git configured correctly? ... yes Database config exists? ... yes All migrations up? ... yes Database contains orphaned GroupMembers? ... no GitLab config exists? ... yes GitLab config up to date? ... yes Log directory writable? ... yes Tmp directory writable? ... yes Uploads directory exists? ... yes Uploads directory has correct permissions? ... yes Uploads directory tmp has correct permissions? ... skipped (no tmp uploads folder yet) Init script exists? ... skipped (omnibus-gitlab has no init script) Init script up-to-date? ... skipped (omnibus-gitlab has no init script) Projects have namespace: ... 2/1 ... yes Redis version >= 2.8.0? ... yes Ruby version >= 2.3.5 ? ... yes (2.5.3) Git version >= 2.18.0 ? ... yes (2.18.1) Git user has default SSH configuration? ... yes Active users: ... 2

Checking GitLab App ... Finished

Checking GitLab subtasks ... Finished