Skip to content
Snippets Groups Projects

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

    • Closed (moved) Issue created by Takaki Hoshikawa @oboenikui

    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

    1. Make some changes to README.md and commit & push. (Call this commit-1)
    2. Create Merge Request.
    3. Make another changes and commit & push. (Call this commit-2)
    4. 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

    1. Move README.md to README.md.bak and commit & push.
    2. 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?

    1. API call is malformed, but the comment is saved to DB.
    2. 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 
    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 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

    Designs

      Child items ...

      2. Activity

      Please register or sign in to reply